New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
deps: update V8 to 12.5 #52651
base: main
Are you sure you want to change the base?
deps: update V8 to 12.5 #52651
Conversation
Major V8 updates are usually API/ABI incompatible with previous versions. This commit adapts NODE_MODULE_VERSION for V8 12.5. Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md
dllexport introduces issues when compiling with MSVC. PR-URL: nodejs#47251 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>
It introduces process hangs on some platforms because Node.js doesn't tear down V8 correctly. Disable it while we work on a solution. Refs: nodejs#47297 Refs: https://bugs.chromium.org/p/v8/issues/detail?id=13902 PR-URL: nodejs#47450 Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Refs: https://bugs.chromium.org/p/v8/issues/detail?id=14221 PR-URL: nodejs#49639 Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
We are not ready to compile with C++20 support yet. This is only a DCHECK that can be removed without affecting the behavior of release builds. PR-URL: nodejs#49639 Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs#49639 Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs#52293 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>
After enabling -std:c++20 on Windows, patch is now much smaller. PR-URL: nodejs#52465 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
On Windows debug builds, it is not allowed to dereference empty iterators. Refs: https://chromium-review.googlesource.com/c/v8/v8/+/5331688 PR-URL: nodejs#52465 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
Namely: - `v8::ObjectTemplate::SetAccessor(v8::Local<v8::String>, ...);` - `v8::ObjectTemplate::SetNativeDataProperty` with `AccessControl` Refs: v8/v8@46c241e Refs: v8/v8@6ec8839 Co-authored-by: Micha毛l Zasso <targos@protonmail.com>
Review requested:
|
@nodejs/platform-windows there's one little error for now:
|
@targos, with these changes the compilation passes (should probably become part of 0b738c9) From f52e086c0142311f1c4b8b2ccf160050d8f2bdd5 Mon Sep 17 00:00:00 2001
From: StefanStojanovic <stefan.stojanovic@janeasystems.com>
Date: Wed, 24 Apr 2024 16:30:39 +0200
Subject: [PATCH 1/1] V8 v12.5 Win fix
---
deps/v8/src/api/api.cc | 6 ++----
deps/v8/src/flags/flag-definitions.h | 3 ++-
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/deps/v8/src/api/api.cc b/deps/v8/src/api/api.cc
index e99e7d9d135..bd2254d82a7 100644
--- a/deps/v8/src/api/api.cc
+++ b/deps/v8/src/api/api.cc
@@ -6323,8 +6323,7 @@ void v8::Object::SetAlignedPointerInInternalFields(int argc, int indices[],
// static
void* v8::Object::Unwrap(v8::Isolate* isolate, i::Address wrapper_obj,
CppHeapPointerTag tag) {
- return i::JSApiWrapper(i::JSObject::cast(i::Tagged<i::Object>(
- reinterpret_cast<i::Address>(wrapper_obj))))
+ return i::JSApiWrapper(i::JSObject::cast(i::Tagged<i::Object>((wrapper_obj))))
.GetCppHeapWrappable(reinterpret_cast<i::Isolate*>(isolate),
static_cast<i::ExternalPointerTag>(tag));
}
@@ -6332,8 +6331,7 @@ void* v8::Object::Unwrap(v8::Isolate* isolate, i::Address wrapper_obj,
// static
void v8::Object::Wrap(v8::Isolate* isolate, i::Address wrapper_obj,
CppHeapPointerTag tag, void* wrappable) {
- return i::JSApiWrapper(i::JSObject::cast(i::Tagged<i::Object>(
- reinterpret_cast<i::Address>(wrapper_obj))))
+ return i::JSApiWrapper(i::JSObject::cast(i::Tagged<i::Object>((wrapper_obj))))
.SetCppHeapWrappable(reinterpret_cast<i::Isolate*>(isolate), wrappable,
static_cast<i::ExternalPointerTag>(tag));
}
diff --git a/deps/v8/src/flags/flag-definitions.h b/deps/v8/src/flags/flag-definitions.h
index 68d98ae63e2..cd469b7537e 100644
--- a/deps/v8/src/flags/flag-definitions.h
+++ b/deps/v8/src/flags/flag-definitions.h
@@ -848,10 +848,11 @@ DEFINE_INT(invocation_count_for_feedback_allocation, 8,
// Tiering: Maglev.
#if defined(ANDROID)
DEFINE_INT(invocation_count_for_maglev, 1000,
+ "invocation count required for optimizing with Maglev")
#else
DEFINE_INT(invocation_count_for_maglev, 400,
-#endif // ANDROID
"invocation count required for optimizing with Maglev")
+#endif // ANDROID
DEFINE_INT(invocation_count_for_maglev_osr, 100,
"invocation count required for maglev OSR")
DEFINE_BOOL(osr_from_maglev, false,
--
2.44.0.windows.1 However, there is another issue I got when running basic debug build sanity check (fails with Release too) Here is the log
Based on the errors, looks like not using |
This comment was marked as outdated.
This comment was marked as outdated.
@StefanStojanovic Not sure what's wrong with your local tests but Windows CI is green! |
This makes me both happy and sad... 馃槀 Hopefully it was something on my machine, I'll see if I can reproduce it again. |
I know, it never ends 馃槄