Closed Bug 1162519 Opened 4 years ago Closed 4 years ago

mach clobber should use rm on Windows if it's winrm

Categories

(Firefox Build System :: General, defect)

Unspecified
Windows
defect
Not set

Tracking

(firefox42 fixed)

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: ted, Assigned: ted)

References

(Depends on 1 open bug)

Details

Attachments

(2 files)

The latest MozillaBuild pre-release includes vlad's winrm tool as rm.exe, which is fast at deleting things:

$ cp -R debug-mozilla-central  debug-mozilla-copy
$ cd mozilla-central/
$ time ./mach clobber

real    0m18.126s
user    0m0.030s
sys     0m0.046s

$ time rm -rf ../debug-mozilla-copy/

real    0m7.461s
user    0m0.015s
sys     0m0.015s

We should use that if it's available. We could detect it during configure or in mach, whatever:
$ rm -h
winrm version 0.4
...

$ rm-msys.exe -h
rm-msys.exe: invalid option -- h
Try `rm-msys.exe --help' for more information.
winrm has been shipping in mozillabuild since version 1.10, released in late July of last year.
bug 1162519 - use winrm for mach clobber on Windows. r?gps
Attachment #8628291 - Flags: review?(gps)
Comment on attachment 8628291 [details]
MozReview Request: bug 1162519 - use winrm for mach clobber on Windows. r?gps

https://reviewboard.mozilla.org/r/12369/#review10873

Cool. FWIW, I think further optimizing mozfile to reduce redundant stat() could bring its implementation within spitting distance of winrm. We could also leverage some ctypes magic to basically inline winrm into mozfile. But that's all for other bugs.
Attachment #8628291 - Flags: review?(gps) → review+
Here's what I don't understand - we replace rm.exe with winrm as part of the packaging process:
http://hg.mozilla.org/mozilla-build/file/050424653d4e/packageit.sh#l24

Shouldn't this already be working without any extra patches?
`mach clobber` uses mozfile.remove(), which does some chmod stuff and then uses shutil.rmtree, all in Python. It doesn't shell out to `rm`.
https://hg.mozilla.org/mozilla-central/rev/a2c6a5972acb
Assignee: nobody → ted
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
I'm still seeing long clobber times even with this patch. Ted, can you confirm that things are better for you?
Flags: needinfo?(ted)
I tested it locally but I haven't been doing regular Windows builds. Are you saying you're seeing long clobber times on your local builds?
Flags: needinfo?(ted)
Yeah.

$ time ./mach clobber

real    2m26.866s
user    0m0.045s
sys     0m0.046s
Does `winrm -h` in your shell produce help text starting with "winrm version 0.4" ?
Attached image objdir deletion
I think I've got a more general perf issue here. This is a screenshot of shift+deleting my objdir. You can see that it sees some pretty horrible perf dropoffs throughout. The HDD activity indicator is solidly lit the entire time.
Depends on: 1182947
My times are from a machine with an SSD, so if you have a spinning disk it's not going to be comparable.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Eh, will fix in the blocking bug.
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
I have a Crucial m4 512GB SSD. I've tried reinstalling the Rapid Storage drivers in case that helps.
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.