Closed Bug 1271448 Opened 9 years ago Closed 9 years ago

Stop saving reftest.log when running locally

Categories

(Testing :: Reftest, defect)

defect
Not set
normal

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
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)
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?
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)
> 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)
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 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+
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Blocks: 1278577
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: