Open Bug 1409441 Opened 2 years ago Updated 3 months ago

Fix some perf issues on the EnterJit path

Categories

(Core :: JavaScript Engine: JIT, defect, P2)

defect

Tracking

()

ASSIGNED
Tracking Status
firefox58 --- wontfix
firefox59 --- ?

People

(Reporter: jandem, Assigned: jandem)

References

(Blocks 1 open bug)

Details

(Keywords: leave-open, perf)

Attachments

(5 files)

I have some small patches that improve my Promise micro-benchmark (see below) from ~205 ms to ~185 ms.

function f() {
    var g = (r) => void 0;
    var count = 1000000;
    var start = Date.now();
    for (var i = 0; i < count; ++i) {
        new Promise(g);
    }
    var stop = Date.now();
    print(stop - start);
}
f();
This moves ActivationEntryMonitor's constructors to the header file. Code for the uncommon cases I moved into init() methods that can stay in the cpp file.
Attachment #8919348 - Flags: review?(nfitzgerald)
Just a minor improvement to avoid calling clearRematerializedFrames if rematerializedFrames_ is nullptr.
Attachment #8919350 - Flags: review?(nicolas.b.pierron)
JitActivation::isProfiling() always returns true so I removed the check in ~JitActivation (this is also asserted in unregisterProfiling).
Attachment #8919352 - Flags: review?(bbouvier)
Attachment #8919355 - Flags: review?(bbouvier)
Comment on attachment 8919352 [details] [diff] [review]
Part 3 - Inline some Activation/JitActivation methods

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

Thanks
Attachment #8919352 - Flags: review?(bbouvier) → review+
Comment on attachment 8919355 [details] [diff] [review]
Part 4 - More inlining

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

Thanks!
Attachment #8919355 - Flags: review?(bbouvier) → review+
Attachment #8919350 - Flags: review?(nicolas.b.pierron) → review+
Comment on attachment 8919348 [details] [diff] [review]
Part 1 - ActivationEntryMonitor

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

LGTM
Attachment #8919348 - Flags: review?(nfitzgerald) → review+
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/88c4af1d92d0
part 1 - Inline ActivationEntryMonitor constructor/destructor. r=fitzgen
https://hg.mozilla.org/integration/mozilla-inbound/rev/714bd2c7dbc7
part 2 - Move null check from clearRematerializedFrames to JitActivation dtor. r=nbp
https://hg.mozilla.org/integration/mozilla-inbound/rev/f6de6926530e
part 3 - Inline some Activation/JitActivation methods. r=bbouvier
https://hg.mozilla.org/integration/mozilla-inbound/rev/acb5cba79c55
part 4 - Inline some more methods. r=bbouvier
Backed out for crashing devtools' devtools/client/performance/test/browser_perf-console-record-08.js on Linux x64 pgo:

https://hg.mozilla.org/integration/mozilla-inbound/rev/60cadd8c8b54cd5d9d1344dd775027145615a362
https://hg.mozilla.org/integration/mozilla-inbound/rev/4019a873f2e6f1522646ffc076339736da51f2f9
https://hg.mozilla.org/integration/mozilla-inbound/rev/9707e60161b7c1dea846508a850dc49686199f5e
https://hg.mozilla.org/integration/mozilla-inbound/rev/44ececec44e21d24b060c0778a2e827aa18b3ee9

Range with failures:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=success&fromchange=96c6513a7f56e6372a5e7fdaa58b180e806eb5c2&filter-searchStr=9486e9f204f409df17c27a298e57313abb58c503&tochange=60cadd8c8b54cd5d9d1344dd775027145615a362

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=137957649&repo=mozilla-inbound

[task 2017-10-18T19:33:51.356Z] 19:33:51     INFO - PROCESS-CRASH | Main app process exited normally | application crashed [@ js::jit::JSJitProfilingFrameIterator::tryInitWithPC]
[task 2017-10-18T19:33:51.357Z] 19:33:51     INFO - Crash dump filename: /tmp/tmpKurrgE.mozrunner/minidumps/0f7208e1-454e-f913-8899-3a3f5c846068.dmp
[task 2017-10-18T19:33:51.358Z] 19:33:51     INFO - Operating system: Linux
[task 2017-10-18T19:33:51.358Z] 19:33:51     INFO -                   0.0.0 Linux 3.13.0-112-generic #159-Ubuntu SMP Fri Mar 3 15:26:07 UTC 2017 x86_64
[task 2017-10-18T19:33:51.359Z] 19:33:51     INFO - CPU: amd64
[task 2017-10-18T19:33:51.359Z] 19:33:51     INFO -      family 6 model 62 stepping 4
[task 2017-10-18T19:33:51.359Z] 19:33:51     INFO -      2 CPUs
[task 2017-10-18T19:33:51.360Z] 19:33:51     INFO - 
[task 2017-10-18T19:33:51.361Z] 19:33:51     INFO - GPU: UNKNOWN
[task 2017-10-18T19:33:51.361Z] 19:33:51     INFO - 
[task 2017-10-18T19:33:51.361Z] 19:33:51     INFO - Crash reason:  SIGSEGV
[task 2017-10-18T19:33:51.362Z] 19:33:51     INFO - Crash address: 0x0
[task 2017-10-18T19:33:51.364Z] 19:33:51     INFO - Process uptime: not available
[task 2017-10-18T19:33:51.364Z] 19:33:51     INFO - 
[task 2017-10-18T19:33:51.365Z] 19:33:51     INFO - Thread 27 (crashed)
[task 2017-10-18T19:33:51.366Z] 19:33:51     INFO -  0  libxul.so!js::jit::JSJitProfilingFrameIterator::tryInitWithPC [IonCode.h:16a80fc4e102 : 360 + 0x0]
[task 2017-10-18T19:33:51.366Z] 19:33:51     INFO -     rax = 0x00007f3030b414ec   rdx = 0x99e88c0200000000
[task 2017-10-18T19:33:51.367Z] 19:33:51     INFO -     rcx = 0x00007ffc3f1e1928   rbx = 0x00007f30214c88d0
[task 2017-10-18T19:33:51.368Z] 19:33:51     INFO -     rsi = 0x00007f303d69af39   rdi = 0x00007f3030b414ee
[task 2017-10-18T19:33:51.369Z] 19:33:51     INFO -     rbp = 0x00007f30214c8780   rsp = 0x00007f30214c8780
[task 2017-10-18T19:33:51.371Z] 19:33:51     INFO -      r8 = 0x00007f30214c88d0    r9 = 0x00007f303fb05f30
[task 2017-10-18T19:33:51.373Z] 19:33:51     INFO -     r10 = 0x0000000000000001   r11 = 0x7fffffffffffffff
[task 2017-10-18T19:33:51.374Z] 19:33:51     INFO -     r12 = 0x00007f3034adf000   r13 = 0x00007f30214c8860
[task 2017-10-18T19:33:51.375Z] 19:33:51     INFO -     r14 = 0x0000000000003043   r15 = 0x00007f3032639800
[task 2017-10-18T19:33:51.376Z] 19:33:51     INFO -     rip = 0x00007f303cf1498b
[task 2017-10-18T19:33:51.377Z] 19:33:51     INFO -     Found by: given as instruction pointer in context
[task 2017-10-18T19:33:51.377Z] 19:33:51     INFO -  1  libxul.so!js::jit::JSJitProfilingFrameIterator::JSJitProfilingFrameIterator [JSJitFrameIter.cpp:16a80fc4e102 : 494 + 0x8]
[task 2017-10-18T19:33:51.379Z] 19:33:51     INFO -     rbx = 0x00007f30214c88d0   rbp = 0x00007f30214c87c0
[task 2017-10-18T19:33:51.380Z] 19:33:51     INFO -     rsp = 0x00007f30214c8790   r12 = 0x00007f3034adf000
[task 2017-10-18T19:33:51.382Z] 19:33:51     INFO -     r13 = 0x00007f30214c8860   r14 = 0x0000000000003043
[task 2017-10-18T19:33:51.383Z] 19:33:51     INFO -     r15 = 0x00007f3032639800   rip = 0x00007f303cf14b5e
[task 2017-10-18T19:33:51.384Z] 19:33:51     INFO -     Found by: call frame info
[task 2017-10-18T19:33:51.385Z] 19:33:51     INFO -  2  libxul.so!JS::ProfilingFrameIterator::iteratorConstruct [Stack.cpp:16a80fc4e102 : 1875 + 0xe]
[task 2017-10-18T19:33:51.385Z] 19:33:51     INFO -     rbx = 0x00007f30214c88b0   rbp = 0x00007f30214c87f0
[task 2017-10-18T19:33:51.386Z] 19:33:51     INFO -     rsp = 0x00007f30214c87d0   r12 = 0x00007f30214c8860
[task 2017-10-18T19:33:51.387Z] 19:33:51     INFO -     r13 = 0x00007f30214c88d0   r14 = 0x00007ffc3f1e0ff0
[task 2017-10-18T19:33:51.387Z] 19:33:51     INFO -     r15 = 0x00007f30214d2970   rip = 0x00007f303d076e0c
[task 2017-10-18T19:33:51.388Z] 19:33:51     INFO -     Found by: call frame info
[task 2017-10-18T19:33:51.388Z] 19:33:51     INFO -  3  libxul.so!JS::ProfilingFrameIterator::ProfilingFrameIterator [Stack.cpp:16a80fc4e102 : 1803 + 0x5]
[task 2017-10-18T19:33:51.389Z] 19:33:51     INFO -     rbx = 0x00007f30214c88b0   rbp = 0x00007f30214c8810
[task 2017-10-18T19:33:51.389Z] 19:33:51     INFO -     rsp = 0x00007f30214c8800   r12 = 0x00007f30214d29c0
[task 2017-10-18T19:33:51.390Z] 19:33:51     INFO -     r13 = 0x00007f30214c88b0   r14 = 0x0000000000000000
[task 2017-10-18T19:33:51.390Z] 19:33:51     INFO -     r15 = 0x00007f30214d2970   rip = 0x00007f303d076f60
[task 2017-10-18T19:33:51.391Z] 19:33:51     INFO -     Found by: call frame info
[task 2017-10-18T19:33:51.391Z] 19:33:51     INFO -  4  libxul.so!MergeStacks(unsigned int, bool, ThreadInfo const&, Registers const&, NativeStack const&, ProfilerStackCollector&) [clone .constprop.441] [clone .cold.459] + 0x157
[task 2017-10-18T19:33:51.392Z] 19:33:51     INFO -     rbx = 0x000000000000001d   rbp = 0x00007f30214d2940
[task 2017-10-18T19:33:51.392Z] 19:33:51     INFO -     rsp = 0x00007f30214c8820   r12 = 0x00007f30214d29c0
[task 2017-10-18T19:33:51.393Z] 19:33:51     INFO -     r13 = 0x00007f30214c88b0   r14 = 0x0000000000000000
[task 2017-10-18T19:33:51.394Z] 19:33:51     INFO -     r15 = 0x00007f30214d2970   rip = 0x00007f303cc8b94b
[task 2017-10-18T19:33:51.395Z] 19:33:51     INFO -     Found by: call frame info
[task 2017-10-18T19:33:51.395Z] 19:33:51     INFO -  5  libxul.so!EnterJit(JSContext*, js::RunState&, unsigned char*) + 0x259
[task 2017-10-18T19:33:51.396Z] 19:33:51     INFO -     rbp = 0x00007f30214d2940   rsp = 0x00007f30214c8868
[task 2017-10-18T19:33:51.397Z] 19:33:51     INFO -     rip = 0x00007f303d69af39
[task 2017-10-18T19:33:51.397Z] 19:33:51     INFO -     Found by: stack scanning
Flags: needinfo?(jdemooij)
Keywords: perf
Priority: -- → P2
(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #9)
> Backed out for crashing devtools'
> devtools/client/performance/test/browser_perf-console-record-08.js on Linux
> x64 pgo:

Sigh. Probably a pre-existing bug with the compiler reordering initialization of JitActivation fields and that confusing the profiler.
I'll start Try-servering and landing this incrementally so if this still happens we can see where it starts...
(In reply to Jan de Mooij [:jandem] from comment #13)
> I'll start Try-servering and landing this incrementally so if this still
> happens we can see where it starts...

The merge happened so now is probably a good time for this.
Flags: needinfo?(jdemooij)
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/960b295f45af
part 1 - Inline ActivationEntryMonitor constructor/destructor. r=fitzgen
Keywords: leave-open

The leave-open keyword is there and there is no activity for 6 months.
:jandem, maybe it's time to close this bug?

Flags: needinfo?(jdemooij)

(In reply to Release mgmt bot [:sylvestre / :calixte] from comment #17)

The leave-open keyword is there and there is no activity for 6 months.
:jandem, maybe it's time to close this bug?

Nope.

Flags: needinfo?(jdemooij)
Attachment #9081566 - Attachment is obsolete: true
Attachment #9081566 - Attachment is obsolete: false
You need to log in before you can comment on or make changes to this bug.