636561 - Reduce UnwantedForeground deallocations to zero.

Status: RESOLVED INCOMPLETE

(Core :: JavaScript Engine, defect)

Description

[Paul Biggar]

After bug 634155, there are 67 Foreground deallocation sites in SpiderMonkey. We should reduce these to as close to zero as possible.

Comment 1

[Andreas Gal :gal]

Are those sites in performance critical paths?

Comment 2

[Paul Biggar]

(In reply to comment #1)
> Are those sites in performance critical paths?

I'm not sure. In general, this happen in sections of code that don't have a context passed to them (in general the same ones with OfftheBooks allocations), for example arenas.

Comment 3

[Andreas Gal :gal]

If its in a slow path immediate deallocation seems desirable. I would suggest we work on this from a profile perspective. Fix anything that is causing us to be slow.

Comment 4

[Paul Biggar]

(In reply to comment #3)
> If its in a slow path immediate deallocation seems desirable.

Do you mean that we _prefer_ Foreground deallocation on the slow path, or that there's no need to change code to allow Foreground deallocation on the slow path?

Comment 5

[Brendan Eich [:brendan]]

Shipping a pointer to the deallocation thread is not free, and we have (AFAICT) not resolved the thread contention issues with some mallocs that may make the whole thing bite back, not in latency to "free" but in throughput.

So yeah, there are reasons to prefer foreground free.

We shouldn't do much of any performance tuning without profile data.

/be

Comment 6

[Paul Biggar]

I've changed the title to reflect the discussion so far. Basically, UnwantedForeground allocations are allocations that have not yet been audited. If ew determine that they belong in the Foreground, we can leave them there by renaming them to Foreground::allocfunc(). Otherwise, we can move them to the background by finding a context to run the allocation through.

Comment 7

[Paul Biggar]

(In reply to comment #6)

s/allocations/deallocations/g above