Hi Ulan,

Thanks for the explanation!

> If the second callback does not increase the heap limit sufficiently 
(e.g. max young generation size ~ 32MB by default) then the GC may crash 
with OOM because it may need to promote young objects into the old 
generation, which checks the limit.

So IIUC, the safe thing to do should be to return a new limit that's 
initial_heap_limit + max_young_generation_size in the second callback. From 
what I can tell, there should two ways to get to the young generation size:

- Save the max_young_generation_size_in_bytes() from the resource 
constraints used to create the isolate.
- Do a GetHeapSpaceStatistics() in the callback, and filter out the one 
with the name "new_space", then use the space_size() of that particular 
item.

Which one would be more preferable? My guess is the first one might be 
larger than the second one, but probably not by lot, and it saves the 
filtering code and  thus is more reliable.

Also, is it  safe to assume that the GC triggered by snapshot generation 
will not invoke the NearHeapLimitCallback more than once? Or should I guard 
against more potential invocations?

Joyee

On Thursday, April 23, 2020 at 3:01:42 PM UTC+8, Ulan Degenbaev wrote:
>
> Hi Joyee,
>
> The heap snapshot indeed uses native memory. The only caveat with taking 
> heap snapshot inside NearHeapLimitCallback is that the callback may be 
> invoked by one of the GCs that are triggered by the heap snapshot 
> generator. So the callback has to be re-entrant (it seems that your PR is 
> already handling this case). If the second callback does not increase the 
> heap limit sufficiently (e.g. max young generation size ~ 32MB by default) 
> then the GC may crash with OOM because it may need to promote young objects 
> into the old generation, which checks the limit.
>
> Other than that it should be safe.
>
> Cheers,
> Ulan.
>
> On Thu, Apr 23, 2020 at 1:50 AM Joyee Cheung <jo...@igalia.com 
> <javascript:>> wrote:
>
>> I had been thinking that taking heap snapshots should not be viable since 
>> this also consumes memory, but it seems to work just fine when I tried to 
>> implement it in Node.js https://github.com/nodejs/node/pull/33010 - 
>> probably because taking the heap snapshot doesn't really consume that much 
>> memory from the V8 heap, it just mostly uses usual native memory ?
>>
>> And the fact that taking heap snapshots triggers GC seem to help 
>> generating multiple snapshots for users to compare, compared to writing 
>> just one when V8 is about to crash which is what 
>> https://github.com/blueconic/node-oom-heapdump does with the OOM 
>> callback. Though thinking about it a bit more, if node-oom-heapdump has 
>> been working, then at least taking heap snapshots is already safe even when 
>> the heap limit is already reached.
>>
>> My question is, is this safe? Or to what extent is this safe?
>>
>> -- 
>> -- 
>> v8-users mailing list
>> v8-u...@googlegroups.com <javascript:>
>> http://groups.google.com/group/v8-users
>> --- 
>> You received this message because you are subscribed to the Google Groups 
>> "v8-users" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to v8-u...@googlegroups.com <javascript:>.
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/v8-users/3bdb87b9-4833-4629-bf98-2ce5a66746cc%40googlegroups.com
>>  
>> <https://groups.google.com/d/msgid/v8-users/3bdb87b9-4833-4629-bf98-2ce5a66746cc%40googlegroups.com?utm_medium=email&utm_source=footer>
>> .
>>
>

-- 
-- 
v8-users mailing list
v8-users@googlegroups.com
http://groups.google.com/group/v8-users
--- 
You received this message because you are subscribed to the Google Groups 
"v8-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-users+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/v8-users/1b3c41d8-8c40-48b2-9086-ce9695bbbb3c%40googlegroups.com.

Reply via email to