Closed Bug 1342273 Opened 7 years ago Closed 7 years ago

Improve frontend tracelogging

Categories

(Developer Documentation Graveyard :: JavaScript, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: shu, Unassigned)

References

Details

Attachments

(1 file)

      No description provided.
Attachment #8840680 - Flags: review?(hv1989)
Comment on attachment 8840680 [details] [diff] [review]
Improve frontend tracelogging.

Review of attachment 8840680 [details] [diff] [review]:
-----------------------------------------------------------------

Cool!
Comment on attachment 8840680 [details] [diff] [review]
Improve frontend tracelogging.

Review of attachment 8840680 [details] [diff] [review]:
-----------------------------------------------------------------

Woops. Forgot to flip the switch
Attachment #8840680 - Flags: review?(hv1989) → review+
Pushed by shu@rfrn.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc682c2db247
Improve frontend tracelogging. (r=h4writer)
https://hg.mozilla.org/mozilla-central/rev/cc682c2db247
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Either this bug or bug 1341061 regressed Octane-CodeLoad quite a bit (10%). It's probably this one.

https://arewefastyet.com/#machine=29&view=single&suite=octane&subtest=CodeLoad&start=1487934027&end=1487997160

Could you take a look?
Flags: needinfo?(shu)
(In reply to Jan de Mooij [:jandem] from comment #7)
> Either this bug or bug 1341061 regressed Octane-CodeLoad quite a bit (10%).
> It's probably this one.
> 
> https://arewefastyet.com/
> #machine=29&view=single&suite=octane&subtest=CodeLoad&start=1487934027&end=14
> 87997160
> 
> Could you take a look?

I didn't find any regressions locally.

I did notice that default shell builds #define JS_TRACE_LOGGING 1. If tracelogging is on, I *do* reproduce a big regression in codeload. Could it be that AWFY is building default shells with tracelogging turned on?
Flags: needinfo?(shu) → needinfo?(hv1989)
We should also check if browser builds are compiling in tracelogging.
(In reply to Shu-yu Guo [:shu] from comment #9)
> We should also check if browser builds are compiling in tracelogging.

They are :/ So our options are backing out, optimizing to zero-overhead (might be hard if we want more fine-grained tracelogging?), or adding another #ifdef. The merge is next week.

Hannes, btw, do we really need TL enabled on beta/release?
(In reply to Jan de Mooij [:jandem] from comment #10)
> (In reply to Shu-yu Guo [:shu] from comment #9)
> > We should also check if browser builds are compiling in tracelogging.
> 
> They are :/ So our options are backing out, optimizing to zero-overhead
> (might be hard if we want more fine-grained tracelogging?), or adding
> another #ifdef. The merge is next week.
> 
> Hannes, btw, do we really need TL enabled on beta/release?

We should not compile with TL default on. It's such an internal JS engine dev feature.
Depends on: 1345145
(In reply to Shu-yu Guo [:shu] from comment #8)
> I did notice that default shell builds #define JS_TRACE_LOGGING 1. If
> tracelogging is on, I *do* reproduce a big regression in codeload. Could it
> be that AWFY is building default shells with tracelogging turned on?

All builds have JS_TRACE_LOGGING enabled to 1. Even in that case the overhead should be not-detectable. One of the reasons is that there was a plan to use it in devtools. I.e. it had to work in release builds also.
Flags: needinfo?(hv1989)
You need to log in before you can comment on or make changes to this bug.