Closed Bug 1105128 Opened 7 years ago Closed 6 years ago

Optimize mozpack.copier.FileCopier.copy()

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: gps, Assigned: gps)

References

Details

Attachments

(5 files, 1 obsolete file)

mozpack.copier.FileCopier.copy() is an operation performed on every build. We should make what it is doing (syncing the state of the filesystem from manifests) as fast as possible.

I wrote some patches to make it a little faster. Stay tuned...
Attached file MozReview Request: bz://1105128/gps (obsolete) —
Attachment #8528767 - Flags: review?(mh+mozilla)
/r/1037 - Bug 1105128 - Avoid excessive path normalization in FileCopier.copy()
/r/1039 - Bug 1105128 - Alias os.path functions in local scope
/r/1041 - Bug 1105128 - Add scandir Python package
/r/1043 - Bug 1105128 - Add scandir to virtualenv
/r/1045 - Bug 1105128 - Incorporate scandir into install manifest processing

Pull down these commits:

hg pull review -r ee1ac151aced450f3fe91d88ab4449a461bcae88
https://reviewboard.mozilla.org/r/1035/#review553

The scandir changes probably need some flushing out on Try and with the virtualenv integration. The first 2 patches should be ready for review.

I hope to fire up my Windows machine and get some performance numbers. I'm optimistic scandir makes install manifest processing significantly faster. If not, we should probably drop scandir since it offers no compelling advantage.
https://reviewboard.mozilla.org/r/1039/#review769

Ship It!

::: python/mozbuild/mozpack/copier.py
(Diff revision 1)
> +        dirname = os.path.dirname

You might as well do from os.path import normpath, join, dirname. You can do it locally or globally, it probably doesn't matter much.
Comment on attachment 8528767 [details]
MozReview Request: bz://1105128/gps

Skipping review of the scandir bits per comment 4
Attachment #8528767 - Flags: review?(mh+mozilla)
https://reviewboard.mozilla.org/r/1039/#review773

::: python/mozbuild/mozpack/copier.py
(Diff revision 1)
> +        dirname = os.path.dirname

I'd rather not. We import mozpath at the global level. I think choosing to alias one over the other globally is a bit odd.
https://reviewboard.mozilla.org/r/1035/#review775

I dropped the scandir patches before landing.
Blocks: 1109409
(In reply to Gregory Szorc [:gps] from comment #8)
> https://reviewboard.mozilla.org/r/1039/#review773
> 
> ::: python/mozbuild/mozpack/copier.py
> (Diff revision 1)
> > +        dirname = os.path.dirname
> 
> I'd rather not. We import mozpath at the global level. I think choosing to
> alias one over the other globally is a bit odd.

You could still have used import in the function.
Attachment #8528767 - Attachment is obsolete: true
The first commits landed and I have no intention of following up with scandir at this time. But someone should eventually. Closing this out because partially landed bugs months old bugs are confusing.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.