588201 - Do not call JS_SetGCParameterForThread in messageManager code

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Alon Zakai (:azakai)]

Removing that call will prevent the underlying tracer bug 588128 from showing itself.

Comment 1

[Alon Zakai (:azakai)]

Attachment: patch

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 466812 [details] [diff] [review]
patch

If this helps, great.
This is a code copied from elsewhere when message manager was implemented.

dmandelin, or someone who knows js engine should review this.

(message manager runs in the main thread, so I assume some better values
are set elsewhere)

Comment 3

[David Mandelin [:dmandelin]]

For posterity, the issue being fixed here is this:

  JS_SetGCParameterForThread(cx, JSGC_MAX_CODE_CACHE_BYTES, X);

sets the tracer code cache size to X bytes for the thread that cx is on. In the case of this bug, that thread is the main thread, so this was reconfiguring the size of the main code cache that is used for almost everything. X was small: 1MB, which easily got filled running benchmarks, causing the JIT cache to flush, requiring recompilation, and thus perf regressions. In particular, a benchmark with a trace-code working set size of >1MB would be expected to see severe perf regressions.

Setting such a small value also constitutes a stress test of the tracer, which apparently exposed the assertion in bug 588128.

Comment 4

[Alon Zakai (:azakai)]

(In reply to comment #3)
> Setting such a small value also constitutes a stress test of the tracer, which
> apparently exposed the assertion in bug 588128.

I just noticed that I still see the assertion in rare cases even with this fix - which is explained by what you just said (i.e. with the patch there is far less stress, but until the underlying bug is fixed, the assertion can still appear).

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

dougt pushed:
http://hg.mozilla.org/mozilla-central/rev/b6cf560ce576