863526 - Attempt to GC before the StoreBuffer overflows

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Terrence Cole [:terrence]]

Attachment: v0

Brian's research in Bug 851057 found that watching for when the store buffer is about to overflow and doing a minor GC as soon as possible after that point is faster than taking occasional overflows. This patch takes the 7/8 fullness ratio from that patch, but implements the logic on the common base class so we get the benefit for all store buffers.

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 739373 [details] [diff] [review]
v0

Review of attachment 739373 [details] [diff] [review]:
-----------------------------------------------------------------

This doesn't actually trigger the operation callback. It seems like you would want to do that.

Comment 2

[Terrence Cole [:terrence]]

Attachment: v1: actually trigger the operation callback.

(In reply to Bill McCloskey (:billm) from comment #1)
> This doesn't actually trigger the operation callback. It seems like you
> would want to do that.

D'oh! I decided to rewrite the implementation rather than copying because I changed the name. Thanks for catching that!

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 739687 [details] [diff] [review]
v1: actually trigger the operation callback.

Review of attachment 739687 [details] [diff] [review]:
-----------------------------------------------------------------

I just noticed that we're doing a function call every time we add something to the store buffer. I wonder if that affects performance.

::: js/src/gc/StoreBuffer.h
@@ +117,5 @@
> + * How full we allow a store buffer to become before we request a MinorGC.
> + *
> + * Note: clang-3.2 segfaults if this is defined as a class variable.
> + */
> +const static double HighwaterRatio = 7.0 / 8.0;

Could this go in the .cpp file?

@@ +358,5 @@
>      JSRuntime *runtime;
>  
>      void *buffer;
>  
> +    bool aboutToOverflow;

I think you need to initialize this in the constructor. Also, you probably want to set it to false in disable().

Comment 4

[Terrence Cole [:terrence]]

(In reply to Bill McCloskey (:billm) from comment #3)
> I just noticed that we're doing a function call every time we add something
> to the store buffer. I wonder if that affects performance.

I expect so. I created them out-of-line so that wouldn't have to do a full recompile every test cycle while creating and debugging them. Now would be a great time to inline them.

Brian is also doing a full unaligned ABI call from ion for each of these(!) which I expect we should also inline.

Comment 5

[Terrence Cole [:terrence]]

Green try run at:
https://tbpl.mozilla.org/?tree=Try&rev=0ccf9f11d4d9

Pushed at:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8696520f1e90

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8696520f1e90