Closed
Bug 1271448
Opened 9 years ago
Closed 9 years ago
Stop saving reftest.log when running locally
Categories
(Testing :: Reftest, defect)
Testing
Reftest
Tracking
(firefox49 fixed)
RESOLVED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: ahal, Assigned: ahal)
References
Details
Attachments
(1 file)
In bug 1271035 gps identified disk I/O as a major cause for reftest slowness. He was able to fix most of it, but the new highest source of I/O in reftest is reftest.log.
To note, we were only saving reftest.log when running via mach, so this won't impact automation in any way:
https://dxr.mozilla.org/mozilla-central/source/layout/tools/reftest/mach_commands.py#204
I don't know why we are doing this given that the same output goes to stdout. And as dholbert pointed out, since bug 1034290 landed, that log contains raw structured logs, which are not at all useful for debugging. I propose we simply stop saving it to boost local reftest performance.
If anyone wishes to keep saving to a log, they can use:
./mach reftest --log-tbpl reftest.log
If they wish this to be the default behaviour they can make a machrc (or .machrc) in either topsrcdir, ~/.mozbuild or $MACHRC. Then add:
[alias]
reftest = reftest --log-tbpl reftest.log
Assignee | ||
Comment 1•9 years ago
|
||
In bug 1271035 gps identified disk I/O as a major cause for reftest slowness.
He was able to fix most of it, but the new highest source of I/O in reftest is
reftest.log.
To note, we were only saving reftest.log when running via mach, so this won't
impact automation in any way:
https://hg.mozilla.org/mozilla-central/file/043082cb7bd8/layout/tools/reftest/mach_commands.py#l204
I don't know why we are doing this given that the same output goes to stdout.
And as dholbert pointed out, since bug 1034290 landed, that log contains raw
structured logs, which are not at all useful for debugging. Given that it is
no longer useful and causes slowness, we should stop saving it.
If anyone wishes to keep saving to a log, they can use:
./mach reftest --log-tbpl reftest.log
If they wish this to be the default behaviour they can make a machrc (or
.machrc) in either topsrcdir, ~/.mozbuild or $MACHRC. Then add:
[alias]
reftest = reftest --log-tbpl reftest.log
Review commit: https://reviewboard.mozilla.org/r/51439/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51439/
Attachment #8750490 -
Flags: review?(james)
Assignee | ||
Comment 2•9 years ago
|
||
James, your name showed up in blame (though I suspect you refactored). Do you have any insight on whether this log was being used for something?
Comment 3•9 years ago
|
||
I'm almost certain dbaron will have an opinion here.
FTR, I like the approach of writing reftest.log from mach and not from JS. That should make bug 1271068 a WONTFIX (assuming we don't care about the performance of non-mach invocations).
Flags: needinfo?(dbaron)
Comment 4•9 years ago
|
||
> I don't know why we are doing this given that the same output goes to stdout.
> And as dholbert pointed out, since bug 1034290 landed, that log contains raw
> structured logs, which are not at all useful for debugging. Given that it is
> no longer useful and causes slowness, we should stop saving it.
Not sure who depends on this, or when it got added to the pipeline. I'm fine with piping stdout to a file if that's faster.
Flags: needinfo?(dbaron)
Assignee | ||
Comment 5•9 years ago
|
||
To clarify, we currently pipe output to both stdout and a file. Afaict, there is no good reason that we're piping to a file (on non-Android), especially since structured logging has made its contents a list of json objects (i.e hard to read).
And for those who want/need to pipe to a file, there's an easy workaround in comment 0.
(In reply to Gregory Szorc [:gps] from comment #3)
> FTR, I like the approach of writing reftest.log from mach and not from JS.
> That should make bug 1271068 a WONTFIX (assuming we don't care about the
> performance of non-mach invocations).
Automation does not go through mach, so this bug will only impact running locally. On automation, there is no reftest.log, though mozharness does write the normal raw log that is uploaded for all structured suites.
Comment 6•9 years ago
|
||
Comment on attachment 8750490 [details]
MozReview Request: Bug 1271448 - Stop saving reftest.log for |mach reftest| perf improvements, r?jgraham
https://reviewboard.mozilla.org/r/51439/#review48693
Sure. I don't think I ever knew why it did this in the first place and irrespective of any speed benefit, this brings reftests in line with other test harnesses.
Attachment #8750490 -
Flags: review?(james) → review+
Comment 8•9 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in
before you can comment on or make changes to this bug.
Description
•