Allowing Feedback vectors keeps strong reference to optimized code

[Kwasi Mensah]

Hi v8-dev,

I was debugging something that was causing me to place extra GCs and saw that it was causing a ton of spurious turbofan recompilation of functions and deopts referencing weak objects. After digging a bit it looks like v8 only keeps a weak reference to optimized code in https://github.com/v8/v8/blame/main/src/objects/feedback-vector.cc#L368

It looks like it's done this from the beginning per https://github.com/v8/v8/commit/11a211ff1b81e8d83d1aa4898743c2efb89603bd#diff-cbddb8a6da7810e9fc3647c9cf4f511410c43d11da9f62873bc33dece03a31f5R305

I'm writing a CLI using node and I'm willing to trade memory for speed. Is this a contribution that makes sense behind an experimental flag?

[Jakob Gruber]

On Wed, Feb 15, 2023 at 1:44 AM Kwasi Mensah <kwasi....@gmail.com> wrote:

Hi v8-dev,

I was debugging something that was causing me to place extra GCs and saw that it was causing a ton of spurious turbofan recompilation of functions and deopts referencing weak objects. After digging a bit it looks like v8 only keeps a weak reference to optimized code in https://github.com/v8/v8/blame/main/src/objects/feedback-vector.cc#L368

Are you sure it would help? The 'deopts referencing weak objects' sound unrelated since the referenced objects are probably not optimized code. And deoptimized TF code cannot be reused, so I don't see how we could avoid the recompilation through a strong reference in the FeedbackVector in this case. Please try your proposal locally first before considering adding a flag.

 

It looks like it's done this from the beginning per https://github.com/v8/v8/commit/11a211ff1b81e8d83d1aa4898743c2efb89603bd#diff-cbddb8a6da7810e9fc3647c9cf4f511410c43d11da9f62873bc33dece03a31f5R305

I'm writing a CLI using node and I'm willing to trade memory for speed. Is this a contribution that makes sense behind an experimental flag?

--
--
v8-dev mailing list
v8-...@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+un...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/5136347a-c0f6-4bf1-b8dc-9ee596755441n%40googlegroups.com.

[Kwasi Mensah]

So there's three sites that needed to be updated to get this to work properly:
    * Strong reference on the feedback vector https://github.com/kwasimensah/node/blob/c5ff019a4e93891106859cb272ded1197a92c7e5/deps/v8/src/objects/feedback-vector.cc#L397
    * Strong reference to the code's dependencies https://github.com/kwasimensah/node/blob/974ab4060fe3eff74dc0a62a5a27d516738f4c55/deps/v8/src/objects/code.cc#L792
    * Tell MarkingVistors to treat these references as strong https://github.com/kwasimensah/node/blob/12478684aab233942e0d5dc24f195930c8a5e59d/deps/v8/src/compiler/pipeline.cc#L1199
        * What you really want is for IsWeakObject to return false https://github.com/kwasimensah/node/blob/7b48713334469818661fe276cf571de9c7899f2d/deps/v8/src/objects/code-inl.h#L612
   
I'm working on a repro that's independent of my project and node but I'm currently seeing a 19% speedup but it's still 7% slower than when I'm not forcing GC between each benchmark run* (I think because the hidden classes for temporary objects that are used in optimized code are lost to GC). I also have to see what tests this breaks.

*I'm purposely forcing GCs because I'm writing a possibly long running node program and want to make sure mistimed GCs don't cause it to waste time on spurious recompiles.

To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/CAH3p7oMFL_3XJ_vGNSyn%2B4i7rRdzv8CO2kToKwVFVmp_cTuzhw%40mail.gmail.com.

[Jakob Gruber]

On Thu, Feb 16, 2023 at 7:22 AM Kwasi Mensah <kwasi....@gmail.com> wrote:

So there's three sites that needed to be updated to get this to work properly:
    * Strong reference on the feedback vector https://github.com/kwasimensah/node/blob/c5ff019a4e93891106859cb272ded1197a92c7e5/deps/v8/src/objects/feedback-vector.cc#L397
    * Strong reference to the code's dependencies https://github.com/kwasimensah/node/blob/974ab4060fe3eff74dc0a62a5a27d516738f4c55/deps/v8/src/objects/code.cc#L792
    * Tell MarkingVistors to treat these references as strong https://github.com/kwasimensah/node/blob/12478684aab233942e0d5dc24f195930c8a5e59d/deps/v8/src/compiler/pipeline.cc#L1199
        * What you really want is for IsWeakObject to return false https://github.com/kwasimensah/node/blob/7b48713334469818661fe276cf571de9c7899f2d/deps/v8/src/objects/code-inl.h#L612

This essentially ensures that optimized Code objects and their entire referenced object graph are never collected (dependencies are never removed and hold strong refs, and all objects referenced by the Code are also strong). I'm sceptical that this is an option in any realistic scenario.. Note optimized code isn't collected as long as it's still referenced by any JSFunction (and hasn't been deoptimized == invalidated). 

 

To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/CA%2Be2j9y6U4g-_YmJe6HX8BmZ4GdFFn6U4U8BmpfiVJnQEx3odw%40mail.gmail.com.

[Kwasi Mensah]

On Thu, Feb 16, 2023 at 2:30 AM Jakob Gruber <jgr...@chromium.org> wrote:

On Thu, Feb 16, 2023 at 7:22 AM Kwasi Mensah <kwasi....@gmail.com> wrote:

So there's three sites that needed to be updated to get this to work properly:
    * Strong reference on the feedback vector https://github.com/kwasimensah/node/blob/c5ff019a4e93891106859cb272ded1197a92c7e5/deps/v8/src/objects/feedback-vector.cc#L397
    * Strong reference to the code's dependencies https://github.com/kwasimensah/node/blob/974ab4060fe3eff74dc0a62a5a27d516738f4c55/deps/v8/src/objects/code.cc#L792
    * Tell MarkingVistors to treat these references as strong https://github.com/kwasimensah/node/blob/12478684aab233942e0d5dc24f195930c8a5e59d/deps/v8/src/compiler/pipeline.cc#L1199
        * What you really want is for IsWeakObject to return false https://github.com/kwasimensah/node/blob/7b48713334469818661fe276cf571de9c7899f2d/deps/v8/src/objects/code-inl.h#L612

This essentially ensures that optimized Code objects and their entire referenced object graph are never collected (dependencies are never removed and hold strong refs, and all objects referenced by the Code are also strong). I'm sceptical that this is an option in any realistic scenario.. Note optimized code isn't collected as long as it's still referenced by any JSFunction (and hasn't been deoptimized == invalidated). 

That's essentially the trade-off I'm willing to make. https://github.com/kwasimensah/gc_removes_optimized_code is a minimal repro of the case I'm trying to optimize. Basically a ton of temporary objects are created while running a job and if a GC happens while none of those objects are alive then we lose the optimized versions of their code and v8 keeps wasting time recompiling them.

To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/CAH3p7oNhPy7vMTN_aOg1Toq_%3DJna82NbYqnWbmKfR-9rZAuzCw%40mail.gmail.com.