New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve directory scanning performance #4426

Merged
merged 5 commits into from Jun 26, 2017

Conversation

Projects
None yet
5 participants
@theotherjimmy
Contributor

theotherjimmy commented Jun 1, 2017

Description

Graph of the first commit compared to master in the next comment.

The remaining commits allow us to correctly elide much processing. In particular, if we delay scanning of features until we try access their respective resource objects, we can avoid scanning some resources by never trying to access their resource objects. Turns out that this optimization saves us ~1/2 of the scan resources time!

Todos

  • /morph test
@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jun 1, 2017

Incremental compile with no changes (seconds):
output

@sg- sg- added the needs: CI label Jun 3, 2017

@0xc0170

0xc0170 approved these changes Jun 5, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jun 5, 2017

retest uvisor

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jun 5, 2017

@mazimkhan Please help with uvisor retest here?

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jun 5, 2017

retest uvisor

@mazimkhan

This comment has been minimized.

Contributor

mazimkhan commented Jun 16, 2017

@0xc0170 There is build failure hence re-running CI will not make it work. Please always have a glance at the logs before re-running CI.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jun 16, 2017

@0xc0170 There is build failure hence re-running CI will not make it work. Please always have a glance at the logs before re-running CI.

I did and I considered the failure to be non related to this patch. Looks like I was wrong, this is the second time, it is related.
@theotherjimmy please look at it. The scanning might be broken as some variables are undefined.

@0xc0170 0xc0170 added needs: work and removed needs: CI labels Jun 16, 2017

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jun 16, 2017

@mazimkhan Could you link me to the test case that's failing? (I can see the CI failure, I just want a link to the code itself) Nevermind. I'm having trouble reproducing the failure though.

@theotherjimmy theotherjimmy force-pushed the theotherjimmy:faster-scan branch 4 times, most recently Jun 16, 2017

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jun 16, 2017

I think this was not ignoring case correctly. Unfortunately, we were depending on "fnmatch.fnmatch" but the code was effectively doing "fnmatch.fnmatchcase". I changed it to do the correct thing.

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jun 16, 2017

Huh, different error.

@theotherjimmy theotherjimmy force-pushed the theotherjimmy:faster-scan branch 2 times, most recently Jun 16, 2017

@theotherjimmy theotherjimmy force-pushed the theotherjimmy:faster-scan branch to 85748db Jun 16, 2017

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jun 16, 2017

I think I got most of the fat out of the directory scanning. We're down to between 15ms and 45ms spent in resource scanning

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jun 16, 2017

@mazimkhan Looks like it's passing now, but not reporting. Nope it reported. My bad.

@theotherjimmy theotherjimmy added needs: CI and removed needs: work labels Jun 19, 2017

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jun 22, 2017

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Jun 22, 2017

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 617

Test failed!

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jun 23, 2017

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Jun 23, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 628

All builds and test passed!

@theotherjimmy theotherjimmy merged commit 86f7cf4 into ARMmbed:master Jun 26, 2017

4 checks passed

Cam-CI uvisor Build & Test Success
Details
ci/morph-test Job has completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@theotherjimmy theotherjimmy deleted the theotherjimmy:faster-scan branch Jun 26, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment