Various improvements to Tracelogger

RESOLVED FIXED in Firefox 49

Status

()

Core
JavaScript Engine: JIT
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: h4writer, Assigned: h4writer)

Tracking

unspecified
mozilla49
Points:
---

Firefox Tracking Flags

(firefox49 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
During the use of Tracelogger this week I introduced some small changes that best should get into the tree.

- Various additions to log places. IonAnalysis, WasmCompilation, CompressSource, RestartLoop
- Mark the creation of Events as TL internal time
- For IonCompilation only report after releasing the helperthread lock. This was causing a false positive I chased for some time.
(Assignee)

Comment 1

2 years ago
Created attachment 8749626 [details] [diff] [review]
Patch
Assignee: nobody → hv1989
Attachment #8749626 - Flags: review?(bbouvier)
Comment on attachment 8749626 [details] [diff] [review]
Patch

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

Nice, thanks!

::: js/src/jit/IonBuilder.cpp
@@ +2463,5 @@
>  
>  IonBuilder::ControlStatus
>  IonBuilder::restartLoop(const CFGState& state)
>  {
> +    TraceLoggerThread* logger = TraceLoggerForMainThread(compartment->runtime()->mainThread()->runtimeFromMainThread());

Even if it's used only once, perhaps you could make an helper mainThread() in IonBuilder, just below names()?

::: js/src/vm/HelperThreads.cpp
@@ +1369,5 @@
>      {
>          AutoUnlockHelperThreadState unlock;
> +
> +        TraceLoggerThread* logger = TraceLoggerForCurrentThread();
> +        AutoTraceLog logCompile(logger, TraceLogger_WasmCompilation);

If you want to be complete, you should add the same thing in wasm::ModuleGenerator::finishFuncDef, which takes care of the case where we can't off-thread compile things.

I've checked, this can't happen with source compression as we always need at least one helper thread. Can this happen with other markers? I guess that GC can't be done in parallel all the time and needs to happen in any cases, so maybe its execution time needs to be captured somewhere else too?
Attachment #8749626 - Flags: review?(bbouvier) → review+

Comment 3

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/9458e02f78d2

Comment 4

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9458e02f78d2
Status: NEW → RESOLVED
Last Resolved: 2 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.