Skip to content

Conversation

wks
Copy link
Collaborator

@wks wks commented Sep 1, 2025

Like #1380, but we remove the old OverCommit, and change the semantics of OverCommit to that of RequestAndOverCommit.

By doing this, the OnAllocationFail will no longer prevent polling from happening.

wks added 4 commits September 1, 2025 15:01
This is useful if the VM cannot trigger GC at most of its allocation
sites.  If allocation over-commits while still triggering GC, the GC
thread can pause the mutators at the nearest safepoints.
@qinsoon
Copy link
Member

qinsoon commented Oct 8, 2025

We discussed this PR. The enum OnAllocationFail could be refactored into boolean flags to reflect the following logic:

  • Can I do a GC now? (at safepoint?)
    • If yes, trigger GC and block.
    • If no, can I overcommit?
      • If yes, overcommit; do we immediately schedule a GC?
        • If yes, schedule a GC.
        • If no, don’t schedule a GC. GC will be scheduled/triggered in the future allocation.
      • If no, return failure (we don’t schedule GC here. The runtime knows the failed allocation, if they want a GC, they can trigger one).

@wks
Copy link
Collaborator Author

wks commented Oct 9, 2025

We discussed this PR. The enum OnAllocationFail could be refactored into boolean flags to reflect the following logic:

* Can I do a GC now? (at safepoint?)
  
  * If yes, trigger GC and block.
  * If no, can I overcommit?
    
    * If yes, overcommit; do we immediately schedule a GC?
      
      * If yes, schedule a GC.
      * If no, don’t schedule a GC. GC will be scheduled/triggered in the future allocation.
    * If no, return failure (we don’t schedule GC here. The runtime knows the failed allocation, if they want a GC, they can trigger one).

I think it is a good idea to simply use boolean fields in AllocationOptions. But there is currently impossible to ask "do we immediately schedule a GC?" because GCTrigger::poll automatically calls self.gc_requester.request() when GC is considered required, and it schedules a collection which will only affect future safepoint executions, but not the current allocation, if we already know we are not at a safepoint. So the decision tree can be simplified as:

  • Call poll. If it didn't trigger GC, allocate as normal. If it triggers GC, then
  • Can we over-commit?
    • If yes, allocate and return.
    • If no, are we at safepoint?
      • If yes, block for GC.
      • If no return 0.

@wks
Copy link
Collaborator Author

wks commented Oct 9, 2025

We discussed this PR. The enum OnAllocationFail could be refactored into boolean flags to reflect the following logic:

* Can I do a GC now? (at safepoint?)
  
  * If yes, trigger GC and block.
  * If no, can I overcommit?
    
    * If yes, overcommit; do we immediately schedule a GC?
      
      * If yes, schedule a GC.
      * If no, don’t schedule a GC. GC will be scheduled/triggered in the future allocation.
    * If no, return failure (we don’t schedule GC here. The runtime knows the failed allocation, if they want a GC, they can trigger one).

Yes. I think it is a good idea to use boolean options.

After a second thought, I think the option can include three independent question:

  • Are we at safepoint?
  • Can we over-commit?
  • Do we poll to trigger GC?

Specifically, we don't ask whether we "immediately schedule a GC" or "schedule a GC later" because GCTrigger::poll automatically calls self.gc_requester.request() to schedule a GC, and whether it affects the current allocation is determined by whether the current allocation is at a safepoint. If yes, we can block for GC; if no, the next safepoint will block.

The concrete decision tree will involve all of the three questions above, but the control flow will be complicated. I think it is better to base the change on the following refactoring:

#1381

This changes the control flow into a straight line style, and it should be easier to inject the three questions at appropriate points of the control flow.

I'll have another look at #1381 and get it merged first. That PR doesn't change the overall semantics except for fixing some obvious bugs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants