nodejs / nan

Native Abstractions for Node.js
MIT License
3.27k stars 501 forks source link

'IdleNotificationDeadline' is deprecated: Use MemoryPressureNotification() #953

Open cbratschi opened 1 year ago

cbratschi commented 1 year ago

Getting the following warning with Node.js 20.x:

../node_modules/nan/nan.h:686:39: warning: 'IdleNotificationDeadline' is deprecated: Use MemoryPressureNotification() to influence the GC schedule. [-Wdeprecated-declarations]
    return v8::Isolate::GetCurrent()->IdleNotificationDeadline(
...

This method will be deprecated soon.

https://chromium.googlesource.com/v8/v8/+/refs/heads/main/include/v8-isolate.h#1295

vrza commented 10 months ago

While nan's IdleNotification could be changed to use MemoryPressureNotification with newer versions of v8, e.g.

#if defined(V8_MAJOR_VERSION) && (V8_MAJOR_VERSION > 11 ||                      \
  (V8_MAJOR_VERSION == 11 && defined(V8_MINOR_VERSION) && V8_MINOR_VERSION >= 3))
  inline bool IdleNotification(int idle_time_in_ms) {
    v8::Isolate::GetCurrent()->MemoryPressureNotification(v8::MemoryPressureLevel::kModerate);
    return true;
  }

It seems to make more sense to deprecate nan's IdleNotification, and perhaps introduce a new shim for MemoryPressureNotification to the nan API.

kkoopa commented 10 months ago

Perhaps. Do you have any more details on what the difference is?

On November 2, 2023 6:18:18 PM GMT+01:00, "Vladimir Vrzić" @.***> wrote:

While nan's IdleNotification could be changed to use MemoryPressureNotification with newer versions of v8, e.g.

#if defined(V8_MAJOR_VERSION) && (V8_MAJOR_VERSION > 11 ||                      \
 (V8_MAJOR_VERSION == 11 && defined(V8_MINOR_VERSION) && V8_MINOR_VERSION >= 3))
 inline bool IdleNotification(int idle_time_in_ms) {
   v8::Isolate::GetCurrent()->MemoryPressureNotification(v8::MemoryPressureLevel::kModerate);
   return true;
 }

It seems to make more sense to deprecate nan's IdleNotification, and perhaps introduce a new shim for MemoryPressureNotification to the nan API.

vrza commented 10 months ago

From reading the v8 API docs, while both IdleNotificationDeadline and MemoryPressureNotification boil down to embedder asking v8 to run GC, the semantics are different.

Concretely the idea behind IdleNotification was for the embedder to notify the engine when they are idle, in order to minimize the chance of GC cycles interrupting actual work. And they would pass an amount of time, a "deadline" for the engine to try and do as much work as possible, and engine would return true if no more GC can be done at that point in time.

On the other hand, the idea behind MemoryPressureNotification is for the embedder to notify the engine that they are running out of memory. Not necessarily when the embedder is idle, this can be called from a separate thread while the isolate is running JS code. The signature is also different, there is no deadline parameter, and no value is returned.

So, while similar, the end user of the API (developer) would likely not just simply replace calls to IdleNotificationDeadline with calls to MemoryPressureNotification without making other changes to their code.