[heap] Align allocation retry loop for oilpan with HeapAllocator (v8) [v8/v8 : main]
Etienne Pierre-Doray (Gerrit)
Etienne Pierre-Doray added 3 comments![]( "Open in Gerrit")
GCConfig::FreeMemoryHandling::kLastResort};Etienne Pierre-DorayThis would trigger the "last resort" GC (or actually only parts of it) the first time we encounter an allocation failure IIUC. That's likely too agressive. I would prefer being uniform with JS here and reuse the same logic: Doing 2 lightweight GCs first and if that's not enough call CollectAllAvailableGarbage(kLastResort). cppgc could have its own implementation of the retry implementation imho if reusing the implementation from JS isn't feasible (or too tricky).
Done -> although this changes behavior a bit without a flag, I don't think it's worth experimenting the extra CollectAllAvailableGarbage.
if reusing the implementation from JS isn't feasible (or too tricky).
I'm looking into this but that seem tricky at this point because of a few considerations/differences (e.g. HeapAllocator supports any thread).
Related details
- Dominik Inführ
- Dominik Inführ
- Michael Lippautz
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominik Inführ (Gerrit)
Dominik Inführ added 2 comments![]( "Open in Gerrit")
if (config.free_memory_handling ==Can we add a separate method CollectAllAvailableGarbage here as well? From the call site it is hard to see what GC is going to be triggered otherwise. I twould also be more uniform with what we have in V8. This feels like two different methods anyway. I suppose this would also allow us to get rid of FreeMemoryHandling::kLastResort and the second GCConfig. Otherwise this CL already looks good to me.
Related details
- Dominik Inführ
- Etienne Pierre-Doray
- Michael Lippautz
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominik Inführ (Gerrit)
Dominik Inführ added 1 comment![]( "Open in Gerrit")
return v8_flags.ineffective_gcs_forces_last_resort &&From the outside it is a bit hard to see that this only returns true with this particular flag. Let's just report `isolate_->heap()->HasConsecutiveIneffectiveMarkCompact()` here and use the flag at the call site in the retry loop.
Etienne Pierre-Doray (Gerrit)
Etienne Pierre-Doray added 1 comment![]( "Open in Gerrit")
if (config.free_memory_handling ==Can we add a separate method CollectAllAvailableGarbage here as well? From the call site it is hard to see what GC is going to be triggered otherwise. I twould also be more uniform with what we have in V8. This feels like two different methods anyway. I suppose this would also allow us to get rid of FreeMemoryHandling::kLastResort and the second GCConfig. Otherwise this CL already looks good to me.
Come to think of it, I think this is better: https://chromium-review.googlesource.com/c/v8/v8/+/7011414
This avoids exposing several new methods on GarbageCollector.
Related details
- Dominik Inführ
- Dominik Inführ
- Michael Lippautz
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Etienne Pierre-Doray (Gerrit)
Etienne Pierre-Doray abandoned this change![]( "Open in Gerrit")
Related details
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |