Closed Bug 895019 Opened 7 years ago Closed 7 years ago

Revive tracelogger

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: h4writer, Assigned: h4writer)

Details

Attachments

(3 files)

It's time to get tracelogger working again. Also more precise and correct around changes from engine. Add baseline and remove methodjit support. Also it will now correctly report when bailing.
Attached patch PatchSplinter Review
I'm still busy to improve the python part to create nice graphs out of it. I probably post that on github or something.
Assignee: general → hv1989
Attachment #777248 - Flags: review?(jdemooij)
Attached patch ImprovementsSplinter Review
Noticed some places that were off, while creating graphs for octane. Those fix it.
Attachment #777568 - Flags: review?(jdemooij)
Comment on attachment 777248 [details] [diff] [review]
Patch

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

::: js/src/ion/CodeGenerator.cpp
@@ +897,5 @@
>      masm.flushBuffer();
>      setOsrEntryOffset(masm.size());
>  
> +#if JS_TRACE_LOGGING
> +    masm.tracelog_log(TraceLogging::INFO_ENGINE_BASELINE);

This should be Ion right?

::: js/src/ion/IonMacroAssembler.cpp
@@ +1251,5 @@
> +    passABIArg(logger);
> +    move32(Imm32(TraceLogging::SCRIPT_START), type);
> +    passABIArg(type);
> +    //TODO: should not get done this way !!!!
> +    movePtr(ImmWord(uintptr_t(script)), rscript);

ImmGCPtr(script)

::: js/src/ion/IonMacroAssembler.h
@@ +933,5 @@
>  
> +#if JS_TRACE_LOGGING
> +    void tracelog_start(JSScript *script);
> +    void tracelog_stop();
> +    void tracelog_log(TraceLogging::Type type);

Style nit: tracelogStart, tracelogStop, tracelogLog.
Attachment #777248 - Flags: review?(jdemooij) → review+
Comment on attachment 777568 [details] [diff] [review]
Improvements

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

Cool, let me know when you have new graphs :)

::: js/src/ion/BaselineBailouts.cpp
@@ +1196,5 @@
>      RootedFunction fun(cx, callee);
>      RootedScript scr(cx, iter.script());
>      AutoValueVector startFrameFormals(cx);
>      while (true) {
> +#if JS_TRACE_LOGGING

It doesn't matter much but I think we should move these to FinishBailoutToBaseline instead.
Attachment #777568 - Flags: review?(jdemooij) → review+
Attached patch Log MinorGCSplinter Review
Attachment #778547 - Flags: review?(jdemooij)
Attachment #778547 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/0f046ccc7b53
https://hg.mozilla.org/mozilla-central/rev/5bdc21ebbc19
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.