Allow managing V8 heap limits

[mraerino]

Applies to denoland/deno#6916

V8 allows setting the max_old_generation_size and max_young_generation_size create parameters through a handy helper: https://chromium.googlesource.com/v8/v8/+/e423f0040333dd3ab410b3c6cab5b71a4f75fa6a/include/v8.h#6409

This PR also exposes the methods to register a callback when the limit is reached, so a crash of V8 can be avoided.

[CLAassistant]

All committers have signed the CLA.

[mraerino]

ok, when trying this out in deno_core i'm getting a linker error...

  = note: Undefined symbols for architecture x86_64:
            "_v8__ResourceConstraints__ConfigureDefaultsFromHeapSize", referenced from:
                rusty_v8::isolate_create_params::raw::ResourceConstraints::configure_defaults_from_heap_size::h0bff31ae2991bf2e in librusty_v8-22814f38384a97cd.rlib(rusty_v8-22814f38384a97cd.3vpkah4o8p3w6byd.rcgu.o)
          ld: symbol(s) not found for architecture x86_64
          clang: error: linker command failed with exit code 1 (use -v to see invocation)

[bartlomieju]

ok, when trying this out in deno_core i'm getting a linker error...

  = note: Undefined symbols for architecture x86_64:
            "_v8__ResourceConstraints__ConfigureDefaultsFromHeapSize", referenced from:
                rusty_v8::isolate_create_params::raw::ResourceConstraints::configure_defaults_from_heap_size::h0bff31ae2991bf2e in librusty_v8-22814f38384a97cd.rlib(rusty_v8-22814f38384a97cd.3vpkah4o8p3w6byd.rcgu.o)
          ld: symbol(s) not found for architecture x86_64
          clang: error: linker command failed with exit code 1 (use -v to see invocation)

@mraerino cargo clean && cargo build fixes usually fixes those errors

[piscisaureus]

ok, when trying this out in deno_core i'm getting a linker error...

Yeah that's a known issue. Once the build script has either downloaded or built libdeno.{a|lib} it will not automatically rebuild/re-download it when there are updates. Patches that fix this are always welcome :)

[piscisaureus]

I don't think this needs to be unsafe.

[mraerino]

yeah, the caller might have to do some casting but i'm thinking of making a generics-based safe impl in Deno for this

[piscisaureus]

Generics based safe is a bit tricky. You'd have to pass an owned pointer like Box<T> or Rc<T>. This thing would need to be dropped when the isolate is disposed or the heap limit callback is removed.

[mraerino]

yup, that's the idea

[mraerino]

i'm still getting a linker error when running cargo test -p deno_core in Deno.

I have this change in my core/Cargo.toml:

-rusty_v8 = "0.7.0"
+rusty_v8 = { path = "../../rusty_v8" }

This is how i'm using it: denoland/deno#6914

I did cargo clean in both projects and deno builds fine, but tests fail with the linker problem. Any clues?

[piscisaureus]

The implementation of this "feature" looks good to me now. The only thing that's missing is a test.

[piscisaureus]

Are you sure you had V8_FROM_SOURCE set when you built it?

…

On Thu, 30 Jul 2020 at 00:08, Marcus Weiner ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/isolate.rs <#427 (comment)>: > @@ -345,6 +361,40 @@ impl Isolate { } } + /// Add a callback to invoke in case the heap size is close to the heap limit. + /// If multiple callbacks are added, only the most recently added callback is + /// invoked. + /// + /// # Safety + /// + /// Don't pass a null-pointer for the callback! + /// Make sure the callback only casts `data` to the type the `data` argument + /// here. + pub unsafe fn add_near_heap_limit_callback( yup, that's the idea — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#427 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABVJEIIU3XB4LJ3F5WOCV3R6CMUZANCNFSM4PKPSVZQ> .

[mraerino]

Are you sure you had V8_FROM_SOURCE set when you built it?

somehow missed that concept. using it now, thanks!

[mraerino]

tests added. could you review again?

[piscisaureus]

LGTM - thanks @mraerino!