mozilla / pluotsorbet

[ARCHIVED] PluotSorbet is a J2ME-compatible virtual machine written in JavaScript.
GNU General Public License v2.0
237 stars 46 forks source link

Re-enable preemption. #1849

Closed brendandahl closed 6 years ago

brendandahl commented 9 years ago

Helps with slower devices and UI responsiveness. As before it seems to hurt startup perf on desktop:

Test Baseline Mean Mean +/- % P Min Max
startupTime 1,185ms 1,292ms 107ms 9.06 WORSE 1,280ms 1,307ms
vmStartupTime 225ms 235ms 9ms 4.16 WORSE 231ms 237ms
bgStartupTime 52ms 49ms -2ms -4.69 SAME 48ms 50ms
fgStartupTime 754ms 809ms 55ms 7.35 WORSE 802ms 819ms
fgRefreshStartupTime 154ms 199ms 45ms 29.23 WORSE 194ms 203ms
fgRestartTime n/a NaNms n/a n/a n/a Infinityms -Infinityms
totalSize 31,656kb 32,219kb 563kb 1.78 SAME 30,923kb 33,084kb
domSize 131kb 131kb 0kb 0.01 SAME 131kb 131kb
styleSize 392kb 437kb 44kb 11.33 WORSE 431kb 445kb
jsObjectsSize 24,161kb 24,595kb 434kb 1.79 WORSE 24,561kb 24,604kb
jsStringsSize 490kb 490kb 0kb 0.04 SAME 490kb 490kb
jsOtherSize 6,288kb 6,352kb 64kb 1.01 SAME 5,066kb 7,212kb
otherSize 193kb 214kb 21kb 10.84 WORSE 203kb 223kb
USS n/a NaNkb n/a n/a n/a Infinitykb -Infinitykb
peakRSS 629,876kb 1,598,196kb 968,320kb 153.73 SAME 1,598,196kb 1,598,196kb
mykmelez commented 9 years ago

This is failing the isolate test:

1.2:1.5 | 0: m 1.2:1.5 | 2 0 a ma 1.2:1.5 | 1: ma 1.2:1.5 | 2: 2 1.2:1.5 | 3: 1 isolate 1.2:1.5 | 4: Isolate ID correct 1.2:1.5 | 5: 4 1.2:1.5 | 6: 5 1.2:1.5 | 7: 1 isolate 1.2:1.5 | 8: ma 1.2:1.5 | 9: ma 3.2:3.5 | 10: 5 0 2 m2 2.2:2.5 | 4 0 1 m1 1.2:1.5 | 3 isolates 1.2:1.5 | 11: ma 1.2:1.5 | 12: 1 isolate 1.2:1.5 | 13: Isolates terminated 1.2:1.5 | 2 1 r mar 1.2:1.5 | 14: mar 1.2:1.5 | 2 2 c marc 1.2:1.5 | 15: marc 1.2:1.5 | 16: Main isolate still running 1.2:1.5 | DONE PASS Same number of lines output. FAIL All lines are contained within output.

brendandahl commented 9 years ago

depends on #1850

mykmelez commented 9 years ago

@brendandahl Can you merge intex into this branch now that #1850 has landed?

brendandahl commented 9 years ago

As discussed on IRC. The startup regression came from the preempiton code being emitted and not the from preemptions happening. To emit less we only now emit checks where OSR's can happen and a preemption check is done in the interpreter loop.

brendandahl commented 9 years ago
Test Baseline Mean Mean +/- % P Min Max
startupTime 1,058ms 978ms -80ms -7.55 SAME 670ms 1,030ms
vmStartupTime 214ms 211ms -3ms -1.51 SAME 209ms 214ms
bgStartupTime 43ms 46ms 3ms 6.76 WORSE 44ms 46ms
fgStartupTime 673ms 614ms -59ms -8.77 SAME 337ms 659ms
fgRefreshStartupTime 127ms 107ms -20ms -16.08 BETTER 79ms 113ms
fgRestartTime n/a NaNms n/a n/a n/a Infinityms -Infinityms
totalSize 68,233kb 67,729kb -504kb -0.74 SAME 63,305kb 68,722kb
domSize 142kb 142kb 0kb 0.02 SAME 142kb 142kb
styleSize 412kb 417kb 5kb 1.13 SAME 403kb 425kb
jsObjectsSize 52,444kb 52,142kb -302kb -0.58 SAME 49,337kb 52,459kb
jsStringsSize 933kb 973kb 40kb 4.27 SAME 952kb 1,165kb
jsOtherSize 14,097kb 13,857kb -240kb -1.70 SAME 12,066kb 14,557kb
otherSize 204kb 198kb -6kb -3.04 BETTER 188kb 207kb
USS n/a NaNkb n/a n/a n/a Infinitykb -Infinitykb
peakRSS 724,400kb 728,940kb 4,540kb 0.63 SAME 728,940kb 728,940kb
mykmelez commented 9 years ago

The test failures are mostly just the unrelated race condition that #1862 is fixing (although the most recent rebuild looks like a hang). But I'm seeing occasional "unresponsive script" hangs when running the startup benchmark with WhatsApp on this branch.

I opened the debugger for the most recent hang and found myself in Thread.prototype.removeFrame, apparently stuck in an infinite loop on:

  for (var fp = this.fp;i32[fp + 1] !== removeFP;) {
    fp = i32[fp + 1];
  }

Which is originally:

  var fp = this.fp;
  while ((i32[fp + FrameLayout.CallerFPOffset]) !== removeFP) {
    fp = i32[fp + FrameLayout.CallerFPOffset];
  }

Because fp is 343040, and so is i32[fp + 1], so it never changes; while removeFP is 343170.

mykmelez commented 6 years ago

I'm closing this pull request (and all others) as this project is no longer active.