Closed Bug 1207871 Opened 7 years ago Closed 7 years ago

Build symbols for large files first

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox45 fixed)

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: gps, Assigned: gps)

References

Details

Attachments

(1 file)

Currently, symbol dumping does an os.walk() and schedules symbol dumping as files are found. This can schedule large files later and leave large files as long poles at the end of the job. Let's exhaustively find files first then sort by file size to help reduce long poles.
Bug 1207871 - Process files in descending file size order; r?ted

Large files take longer to process. Scheduling large files after smaller
files means there is a higher chance a large file may be a long pole
during processing.

This commit changes the scheduling logic to exhaustively obtain the set
of files to be processed. It then sorts them by descending file size and
schedules them in the resulting order, thus minimizing the chances for a
large file to be the long pole holding up processing completion.

On my machine this doesn't change wall execution time. However,
automation may be different. And the logic of the new behavior is sound.
Attachment #8665213 - Flags: review?(ted)
Comment on attachment 8665213 [details]
MozReview Request: Bug 1207871 - Process files in descending file size order; r?ted

Bug 1207871 - Process files in descending file size order; r?ted

Large files take longer to process. Scheduling large files after smaller
files means there is a higher chance a large file may be a long pole
during processing.

This commit changes the scheduling logic to exhaustively obtain the set
of files to be processed. It then sorts them by descending file size and
schedules them in the resulting order, thus minimizing the chances for a
large file to be the long pole holding up processing completion.

On my machine this doesn't change wall execution time. However,
automation may be different. And the logic of the new behavior is sound.
Comment on attachment 8665213 [details]
MozReview Request: Bug 1207871 - Process files in descending file size order; r?ted

https://reviewboard.mozilla.org/r/20157/#review20381

This is generally OK, although I think with a little tweak you could avoid all the changes in the tests. It'd also be nice to have an explicit unit test for this behavior...

::: toolkit/crashreporter/tools/symbolstore.py:1016
(Diff revision 2)
> +            files.add(f)

Could we just move this loop up into Dumper.Process, which would avoid all the rewriting you're doing in the tests below?
Attachment #8665213 - Flags: review?(ted) → review+
Assignee: nobody → gps
I fixed the one issue from my review, added a unit test, and landed this.
https://hg.mozilla.org/mozilla-central/rev/c7d560f6e228
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.