Open Bug 1162272 Opened 9 years ago Updated 2 years ago

Fix our octane-mandreel-latency performance, or better yet, get the benchmark fixed or removed or something

Categories

(Core :: JavaScript Engine, defect)

defect

Tracking

()

Tracking Status
firefox40 --- affected

People

(Reporter: bhackett1024, Unassigned)

Details

OK, so earlier today a change was committed to v8 that nearly tripled their score on octane-mandreel-latency.  The change is be9570027f34f8dbf60a7b530fe2499ff52c1a3d, associated with bug 470930, which I am unable to access.  I didn't look at the patch but looked at the notes, which is something about allowing the parser to abort lazy parsing and restart with a full parse, or something.  This is a feature we already have, btw, since our lazy parser doesn't handle all the crazy JS features which the full parser does.  Anyways, if I disable lazy parsing then our score on octane-mandreel-latency goes up by 2.5x.  Since our current score is considerably higher than v8's earlier score, I think we'd be faster than v8 on mandreel-latency by making this change.

So, what's going on?  Mandreel-latency's score is based on the sum of the squared pause times measured between certain parts of the benchmark.  On my machine, the first pause is like 200ms, then the rest are like 0 or 1 ms.  So the benchmark score is entirely determined by that first pause time.  Since this is the first time we run the code in the benchmark, this is where we do all our lazy parsing, and that occupies a large fraction of the time taken.  If we compile things ahead of time then this time isn't counted against our score in the benchmark.

About 88% of our lazy-function-parsing time before that first "pause" time measurement is taken up by a single function, global_init(), which is about 150 thousand lines of straight line code calling various functions that takes 105ms to parse on my machine.  This isn't a run once closure or anything, and it isn't obvious from the code that this function will end up executing at any point.  Suppose we used full parsing on this function when handling the compilation unit, instead of lazy parsing.  The advantages of doing so:

- If the function eventually runs, we will have saved the time used up lazily parsing it.  From prior experience, this tends to be about 25% of the time used for full parsing.
- Our score on this benchmark will improve.

The disadvantages of doing so:

- If the function never runs, we will have wasted the time spent fully parsing instead of lazily parsing it.
- If the function never runs, we will have wasted a lot of memory on the JSScript for it.
- Whether the function runs or not, parsing latency / browser max pause time will go up.  This might not be as big a deal once we have incremental parsing or can parse generic <script> tags off thread.

The disadvantages are clearly worse than the advantages of making this change.  So I think octane-mandreel-latency is offering us a perverse incentive: by making the browser worse, we can improve our score on this benchmark.  This harkens back to the bad old days of benchmark gaming, and I think that whoever at google is responsible for this benchmark should either fix it or remove it.  Failing that, we could do something stupid here to improve our score on it.
I filed https://github.com/chromium/octane-benchmark/issues/29 against the octane benchmark for this issue.
Should this get "PERF" key word?
For what it is worth, this bug got a mention in the "Retiring Octane" blog post:
  https://v8project.blogspot.de/2017/04/retiring-octane.html
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.