Open Bug 1080776 Opened 10 years ago Updated 2 years ago

Create optimization level at warmupcount 100 that doesn't inline scripts

Categories

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

defect

Tracking

()

People

(Reporter: h4writer, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Currently we only start to compile IonMonkey at warmup count 1000. This has been lowered from 10000, so that is already a big gain. But we might want to lower this even more.

This warmupcount is an heuristic we use to decide when compilation overhead will most likely be lower than the gain in performance. Now the reason we can't lower the warm up count, is because compilation has some "serious" overhead. Now the biggest overhead is for scripts that are (1) very big or (2) inline a lot of functions.

The idea is to create a "tier"/optimization level where the overhead of compilation is really small, but is still faster than baseline. I.e. makes sense to compile this. Let's say at warmup count 100. That makes it possible to run already in IonMonkey (slower tier), before we get to warmup count 1000 and decide to do a full ionmonkey compilation which also might take longer.

Ionmonkey code is 3x faster than baseline code when doing no optimizations (so no inlining). And importantly compilation takes only 0 - 0.04ms overhead. (With full ionmonkey being 0 - 21ms)

Note:
This won't improve benchmark score. This will improve speed for code that is moderate hot.
In other words. This won't improve our topspeed, but make us faster fast.
Blocks: 1078273
Attached patch DraftSplinter Review
I've tried this already a few times, but always got regressions on one or multiple benchmarks. Now with most blockers fixed, the last regression was on octane-mandreel. In this patch I was able to fix the problem. I'll create depending bugs on this for the issues I noticed:

1) Start compiling higher tier as soon as hitting the warmup count. Don't wait for lower tier to finish compilation.

2) Save compiling tier level in JSScript::ion to be able to quickly check when a higher tier can start compiling.

3) During linking, notice if another tier is compiling and update IonScript::recompiling_ accordingly.

4) During linking, throw away if a higher tier was compiled quicker. (Just for sanity).

5) Make it possible to disable a tier for maxScriptSize.

(This is mostly for myself. Since I have some other work I need to finish first, before I can go further with this. And I don't want to forget all this. Took me hours investigation to find these issues.)
(In reply to Hannes Verschore [:h4writer] from comment #0)
> Note:
> This won't improve benchmark score. This will improve speed for code that is
> moderate hot.
> In other words. This won't improve our topspeed, but make us faster fast.

Wouldn't this be visible on Sunspider?
In some cases our compilation pipeline is full, are you thinking of making this compilation synchronous?
(In reply to Nicolas B. Pierron [:nbp] from comment #2)
> Wouldn't this be visible on Sunspider?

I don't consider that as topspeed ;). Had some fun playing and it might be possible to generate a improvement on SunSpider on this. Adjusting heuristics I could get to 161ms (164ms without patch). The biggest problem to getting higher gains is IonBuilder. Making that faster with this patch will probably lower this score even more!

> In some cases our compilation pipeline is full, are you thinking of making
> this compilation synchronous?

No not at all. Our compilation pipeline is not delayed that often. 

For regular compilation on octane:
> times happened    / # of compilations already busy when adding another IonBuilder
> 2382x                  0  
> 1276x                  1 
>  398x                  2 
>  118x                  3 
>   44x                  4 
>   30x                  5
Note: allowing 7 background ion compilations threads gives some improvements, but decreases score on splay of 15%. Which is a 2% regression overall.

Compilation happens mostly in bursts and is actually mostly empty (except for TypeScript). So I don't think making it synchronous will improve the situation. It should deteriorate our score actually.
Priority: -- → P5
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.