1280648 - Tracelogger: don't cache based on pointer to CompilerOptions

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Hannes Verschore [:h4writer]]

CompilerOptions pointer is not unique to the data we want to store. As a result we cannot use the pointer as an id for the cache. For now just allow to not cache it.

Comment 1

[Hannes Verschore [:h4writer]]

Attachment: Patch

1) add a way to insert a textId to text without a key to cache the data
2) Don't cache the CompileOptions anymore, since the pointers are not unique!

Note: I'm not entirely sure, but if a JSScript* can move we will also need to opt-out and create follow-up to clear the cache every time we move JSScripts. Not sure about the status. We have a Handle to JSScript, but also use raw JSScript pointers?

Comment 2

[Hannes Verschore [:h4writer]]

Sfink indicated that JSScript* is not movable and will be one of the last thing to get movable. So we are currently still safe. There is now also HashMaps that support the moving. We can possibly update our cache with that eventually.

Comment 3

[Jan de Mooij [:jandem]]

(In reply to Hannes Verschore [:h4writer] from comment #2)
> Sfink indicated that JSScript* is not movable and will be one of the last
> thing to get movable. So we are currently still safe. There is now also
> HashMaps that support the moving. We can possibly update our cache with that
> eventually.

I don't think that's true - compacting GC can move scripts. See bug 1259180.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a942a270777
Tracelogger: Don't cache based on pointers to movable gc things, r=bbouvier

Comment 5

[Hannes Verschore [:h4writer]]

(In reply to Jan de Mooij [:jandem] from comment #3)
> (In reply to Hannes Verschore [:h4writer] from comment #2)
> > Sfink indicated that JSScript* is not movable and will be one of the last
> > thing to get movable. So we are currently still safe. There is now also
> > HashMaps that support the moving. We can possibly update our cache with that
> > eventually.
> 
> I don't think that's true - compacting GC can move scripts. See bug 1259180.

I also removed the caching for JSScript and opened bug 1281156 for using the cache again for JSScript.

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7a942a270777