1207871 - Build symbols for large files first

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Gregory Szorc [:gps]]

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.

Comment 1

[Gregory Szorc [:gps]]

Attachment: 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 2

[Gregory Szorc [:gps]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4790039cf010

Comment 3

[Gregory Szorc [:gps]]

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 4

[Gregory Szorc [:gps]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=26d2e20788f4

Comment 5

[(not currently active) Ted Mielczarek]

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?

Comment 6

[(not currently active) Ted Mielczarek]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d1dca540b707

Comment 7

[(not currently active) Ted Mielczarek]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c7d560f6e228adc9beeca962b1f9ed3682b4dc5d
Bug 1207871 - Process files in descending file size order; r=ted

Comment 8

[(not currently active) Ted Mielczarek]

I fixed the one issue from my review, added a unit test, and landed this.

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c7d560f6e228

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/c7d560f6e228

Comment 11

[Carsten Book [:Tomcat]]

removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion