External memory pressure: Add a collect threshold#3957
External memory pressure: Add a collect threshold#3957obastemur wants to merge 1 commit intochakra-core:masterfrom
Conversation
uncollectedExternalBytes weren't in any specific use. Put a basic threashold for external data and improve handling the requests
obastemur
force-pushed
the
imp_ext_collect
branch
from
b5ce304 to
50e359a
Compare
| // do fast large allocations in a row, normal GC might not kick in. Let's force the GC | ||
| // here if we need to collect anyhow. | ||
|
|
||
| if (collectOnFail || this->autoHeap.uncollectedExternalBytes > KB128 || size >= KB64) |
There was a problem hiding this comment.
so, if I'm reading this correctly, if ever we end up in a place where uncollectedExternalBytes is > 128KB, every call to this function will trigger a GC. Is there some way we can avoid this? E.g., is there an API to "suggest a GC", and let some centralized GC logic worry about when to schedule that?
There was a problem hiding this comment.
No it won’t trigger GC. It will check whether it’s the time to do GC by checking other heuristics as is. If it really collects, well, uncollectedExternalBytes will be set to 0
There was a problem hiding this comment.
sorry, not following why uncollectedExternalBytes will be 0. Won't they only be 0 if GC could actually collect those?
There was a problem hiding this comment.
How do we plan to do that? (Since some of it could be really external) and the only reason for this parameter to be exist is that we could measure the external memory pressure and try GC
There was a problem hiding this comment.
sorry, not following. I have two questions:
- What is
uncollectedExternalBytesand when does it go to 0? - Are we ever subject to a condition where
uncollectedExternalBytes > KB128and callingCollectNowwill trigger GCs that are unable to reduceuncollectedExternalBytes? (i.e., does this change subject us to overly-aggressive GC?)
There was a problem hiding this comment.
uncollectedExternalBytes symbolize an extra pressure number. i.e. host app allocated a piece of memory and notified us with the size.
At the moment, we do try to GC, regardless what uncollectedExternalBytes is..
So what I did is not limited to putting a 128KB threshold. Whenever requestAlloc fails, we do actually try to GC, so adding up uncollectedExternalBytes and trying again was waste of cpu tick. Cleaned up there a bit and put a threshold.
|
@mike-kaufman @VSadov Has this scenario been sufficiently covered by changes to memory policy or should we consider merging this as well? |
3 participants
uncollectedExternalBytesweren't in any specific use. Put a basic threashold for external data and improve handling the requests