913203 - OOM on emscripten-compiled sqlite with ion parallel compilation [regression]

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect, P3)

Description

[Alon Zakai (:azakai)]

Attachment: sqlite

The attached sqlite benchmark OOMs with

js --ion-parallel-compile=pon src.c.o.js

after taking several GB, but works when parallel compilation is off. Very large amounts of memory are used in the browser as well, where parallel compilation is also on.

This is a regression, for example in beta things still work fine.

Comment 1

[Alon Zakai (:azakai)]

oops, that should have been

js --ion-parallel-compile=on src.c.o.js

Comment 2

[Luke Wagner [:luke]]

Is this compiled with outlining on?  Are there several very-large functions next to each other (presumably there are, iirc, because of the "biggest functions first" sorting).

Comment 3

[Alon Zakai (:azakai)]

Sorry, I should have added, this is not asm.js code, it is normal js. No outlining (we only outline asm.js) or AOT. The OOM happens during execution.

Comment 4

[Luke Wagner [:luke]]

But there are still still some honkin' big functions, I assume?  They shouldn't get to Ion so it's weird to see an OOM.

Comment 5

[Alon Zakai (:azakai)]

I'm bisecting now.

Comment 6

[Alon Zakai (:azakai)]

Bisected to bug 886957.

Comment 7

[Jan de Mooij [:jandem]]

OK, needinfo? from Brian based on comment 6 :)

Comment 8

[Brian Hackett [Laid off!]]

Have any reports of OOM's on actual games / other websites come up from this?  All bug 886957 did was increase compilation thresholds, and without any way to know when we're nearly out of memory, attempting to dial the threshold down to avoid OOM is really just tuning the browser for one specific system.

Comment 9

[Alon Zakai (:azakai)]

I am not aware of other reports. In general most people ship better-optimized code than this, so this might be an extreme case. But my guess is that if we see cycling 3GB spikes of memory as we see here, then smaller applications are likely seeing similar cycles, just smaller, but still that sounds bad.

Comment 10

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Iain, do you mind rechecking this potential OOM issue?

Comment 12

[Iain Ireland [:iain]]

This is very similar to bug 1066759. With --no-ion, this script eventually runs to completion. With ion enabled, it takes longer to print the first output ("create table : took 603 ms" vs "create table : took 319 ms"), and then is terminated by the OOM killer halfway through execution.

Given that both scripts are very large, poorly optimized Emscripten'd versions of sqlite3, it seems pretty likely that they're the same issue. Indeed, in both cases we OOM while ion-building "sqlite3VdbeExec", which is 176K of bytecode, has 5000+ locals and args, and has at least 150 inlineable call sites. This function appears to be the interpreter for sqlite3's internal virtual machine: [https://github.com/smparkes/sqlite/blob/8caf9219240123fbe6cff67b1e0da778c62d7621/src/vdbe.c#L546](over 5000 lines of C).

In bug 886957, bhackett said:

Per the formula in bug 807464 comment 3 a script of length 100000 should take a bit less than a second to compile, which isn't bad and of course can be canceled by the main thread when necessary.

In this case, it is not possible for the main thread to cancel the compilation, because we OOM during IonBuilder itself, and haven't moved the compilation off-thread yet.

We used to have a size limit for off-thread compiles (that would be checked before we started IonBuilder) but we removed it in bug 1010417.

I don't know how often we hit this issue in the wild. I hope it is rarely. If we are interested in improving IonBuilder's performance, though, it might be worth digging into this a little deeper; we've obviously run into some part of IonBuilder that doesn't scale linearly.

Comment 13

[Iain Ireland [:iain]]

WarpBuilder runs offthread now. It also looks like we might abort before starting the compilation. Either way, this no longer crashes.