Commit becce45b authored by Dominik Inführ's avatar Dominik Inführ Committed by V8 LUCI CQ

[json, parsing] Switch to internal GC callbacks

We have two different kinds of GC prologues/epilogues. The user-exposed
one in Heap and the internal one in LocalHeap. During parsing and in
the JSON parser we were using the former. While this is definitely
correct and at the time of implementation it was the only existing
mechanism, I believe the internal callbacks are now a better fit for
these use cases.

Internal callbacks are simpler since they don't allow allocations, which
allows us to run them during the GC safepoint. The user-exposed
interfaces are allowed to allocate and are run either before or after
the safepoint. Such allocations could cause recursive GCs, which is
impossible for internal callbacks.

Bug: v8:12545
Change-Id: Ie697556cec9aa77b2f70704445aa5bd58e0a381a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3435188Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78971}
parent ed8e0f41
...@@ -230,8 +230,8 @@ JsonParser<Char>::JsonParser(Isolate* isolate, Handle<String> source) ...@@ -230,8 +230,8 @@ JsonParser<Char>::JsonParser(Isolate* isolate, Handle<String> source)
chars_may_relocate_ = false; chars_may_relocate_ = false;
} else { } else {
DisallowGarbageCollection no_gc; DisallowGarbageCollection no_gc;
isolate->heap()->AddGCEpilogueCallback(UpdatePointersCallback, isolate->main_thread_local_heap()->AddGCEpilogueCallback(
v8::kGCTypeAll, this); UpdatePointersCallback, this);
chars_ = SeqString::cast(*source_).GetChars(no_gc); chars_ = SeqString::cast(*source_).GetChars(no_gc);
chars_may_relocate_ = true; chars_may_relocate_ = true;
} }
...@@ -317,7 +317,8 @@ JsonParser<Char>::~JsonParser() { ...@@ -317,7 +317,8 @@ JsonParser<Char>::~JsonParser() {
// Check that the string shape hasn't changed. Otherwise our GC hooks are // Check that the string shape hasn't changed. Otherwise our GC hooks are
// broken. // broken.
SeqString::cast(*source_); SeqString::cast(*source_);
isolate()->heap()->RemoveGCEpilogueCallback(UpdatePointersCallback, this); isolate()->main_thread_local_heap()->RemoveGCEpilogueCallback(
UpdatePointersCallback, this);
} }
} }
......
...@@ -312,8 +312,7 @@ class JsonParser final { ...@@ -312,8 +312,7 @@ class JsonParser final {
static const int kInitialSpecialStringLength = 32; static const int kInitialSpecialStringLength = 32;
static void UpdatePointersCallback(v8::Isolate* v8_isolate, v8::GCType type, static void UpdatePointersCallback(void* parser) {
v8::GCCallbackFlags flags, void* parser) {
reinterpret_cast<JsonParser<Char>*>(parser)->UpdatePointers(); reinterpret_cast<JsonParser<Char>*>(parser)->UpdatePointers();
} }
......
...@@ -337,20 +337,17 @@ class RelocatingCharacterStream final ...@@ -337,20 +337,17 @@ class RelocatingCharacterStream final
RelocatingCharacterStream(Isolate* isolate, size_t pos, TArgs... args) RelocatingCharacterStream(Isolate* isolate, size_t pos, TArgs... args)
: UnbufferedCharacterStream<OnHeapStream>(pos, args...), : UnbufferedCharacterStream<OnHeapStream>(pos, args...),
isolate_(isolate) { isolate_(isolate) {
isolate->heap()->AddGCEpilogueCallback(UpdateBufferPointersCallback, isolate->main_thread_local_heap()->AddGCEpilogueCallback(
v8::kGCTypeAll, this); UpdateBufferPointersCallback, this);
} }
private: private:
~RelocatingCharacterStream() final { ~RelocatingCharacterStream() final {
isolate_->heap()->RemoveGCEpilogueCallback(UpdateBufferPointersCallback, isolate_->main_thread_local_heap()->RemoveGCEpilogueCallback(
this); UpdateBufferPointersCallback, this);
} }
static void UpdateBufferPointersCallback(v8::Isolate* v8_isolate, static void UpdateBufferPointersCallback(void* stream) {
v8::GCType type,
v8::GCCallbackFlags flags,
void* stream) {
reinterpret_cast<RelocatingCharacterStream*>(stream) reinterpret_cast<RelocatingCharacterStream*>(stream)
->UpdateBufferPointers(); ->UpdateBufferPointers();
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment