Issue 12808 in v8: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared

37 views
Skip to first unread message

seth.… via monorail

unread,
Apr 18, 2022, 5:22:55 PM4/18/22
to v8-re...@googlegroups.com
Status: Assigned
Owner: seth....@microsoft.com
Type: FeatureRequest

New issue 12808 by seth....@microsoft.com: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808

Problem and potential solution are described in v8-dev discussion here: https://groups.google.com/g/v8-dev/c/21ScFo2Dha0/m/eNxuJ5dXAgAJ

--
You received this message because:
1. The project was configured to send all issue notifications to this address

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

Git Watcher via monorail

unread,
May 11, 2022, 10:28:13 AM5/11/22
to v8-re...@googlegroups.com

Comment #1 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c1

The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/2507217839435a03266c79a72dd9432cec8e1b8a

commit 2507217839435a03266c79a72dd9432cec8e1b8a
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Fri May 06 17:14:43 2022

Improve Script reuse in isolate compilation cache, part 1

Once the root SharedFunctionInfo for any Script gets its bytecode
flushed, the Isolate's compilation cache currently evicts that entry, to
reduce memory usage. However, the associated Script is likely still
alive, since scripts often declare functions which outlive the initial
evaluation of the script. If an identical script is loaded later, a
duplicate Script is created for it, which can waste memory.

In this change, I propose that the compilation cache keys can refer
weakly to the Script. When the root SharedFunctionInfo gets old, instead
of deleting the cache entry entirely, we can just drop the strong
reference to the SharedFunctionInfo. A subsequent lookup in the cache
will retrieve the Script instead of the root SharedFunctionInfo,
indicating an opportunity to save some memory by reusing the existing
Script.

Eventually, all callers to CompilationCache::LookupScript should reuse
the Script if possible. This change implements only the easy case of
reusing the Script for synchronous parsing. Follow-up changes will be
required for the TODO comments left by this change.

Bug: v8:12808
Change-Id: Ia8b0389441a682de9a43e73329049fd2e7835d3d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3597106
Reviewed-by: Toon Verwaest <verw...@chromium.org>
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
Reviewed-by: Leszek Swirski <les...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80472}

[modify] https://crrev.com/2507217839435a03266c79a72dd9432cec8e1b8a/src/objects/hash-table.h
[modify] https://crrev.com/2507217839435a03266c79a72dd9432cec8e1b8a/src/logging/counters-definitions.h
[modify] https://crrev.com/2507217839435a03266c79a72dd9432cec8e1b8a/test/cctest/test-serialize.cc
[modify] https://crrev.com/2507217839435a03266c79a72dd9432cec8e1b8a/test/cctest/test-api.cc
[modify] https://crrev.com/2507217839435a03266c79a72dd9432cec8e1b8a/src/objects/hash-table-inl.h
[modify] https://crrev.com/2507217839435a03266c79a72dd9432cec8e1b8a/src/codegen/compiler.cc
[modify] https://crrev.com/2507217839435a03266c79a72dd9432cec8e1b8a/src/codegen/compilation-cache.cc
[modify] https://crrev.com/2507217839435a03266c79a72dd9432cec8e1b8a/src/execution/isolate.h
[modify] https://crrev.com/2507217839435a03266c79a72dd9432cec8e1b8a/src/objects/compilation-cache-table.h
[modify] https://crrev.com/2507217839435a03266c79a72dd9432cec8e1b8a/test/cctest/heap/test-heap.cc
[modify] https://crrev.com/2507217839435a03266c79a72dd9432cec8e1b8a/src/execution/isolate.cc
[modify] https://crrev.com/2507217839435a03266c79a72dd9432cec8e1b8a/src/objects/compilation-cache-table.cc
[modify] https://crrev.com/2507217839435a03266c79a72dd9432cec8e1b8a/src/heap/heap.cc
[modify] https://crrev.com/2507217839435a03266c79a72dd9432cec8e1b8a/src/codegen/compilation-cache.h
[modify] https://crrev.com/2507217839435a03266c79a72dd9432cec8e1b8a/src/objects/compilation-cache-table-inl.h

Git Watcher via monorail

unread,
May 12, 2022, 7:21:12 AM5/12/22
to v8-re...@googlegroups.com

Comment #2 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c2


The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/dfc5493170e035bcd0c9bc5e29acdfbc6fc0617b

commit dfc5493170e035bcd0c9bc5e29acdfbc6fc0617b
Author: Camillo Bruni <cbr...@chromium.org>
Date: Thu May 12 11:20:34 2022

[web_tests] Skip v8-code-cache.html test for future rebaselining

Bug: v8:12808
Change-Id: I3f081ed2ef17f0f9cd7b48230c97699fd284e9f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3643700
Reviewed-by: Leszek Swirski <les...@chromium.org>
Commit-Queue: Camillo Bruni <cbr...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1002578}

[modify] https://crrev.com/dfc5493170e035bcd0c9bc5e29acdfbc6fc0617b/third_party/blink/web_tests/TestExpectations

Git Watcher via monorail

unread,
May 12, 2022, 6:06:37 PM5/12/22
to v8-re...@googlegroups.com

Comment #3 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c3


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/c8848cf49338c354786b72d83a3a2a4ade2f997a

commit c8848cf49338c354786b72d83a3a2a4ade2f997a
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Thu May 12 21:24:35 2022

Refactor CompilationSubCache

CompilationSubCache has some complexity regarding generations of tables
which is only used by one subclass, CompilationCacheRegExp. This change
adjusts the class hierarchy so that classes only contain the necessary
member functions.

Bug: v8:12808
Change-Id: I4f4cf15bbf9b80c2de0c18aea82a0c238804759d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3629603
Reviewed-by: Leszek Swirski <les...@chromium.org>
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#80506}

[modify] https://crrev.com/c8848cf49338c354786b72d83a3a2a4ade2f997a/src/codegen/compilation-cache.cc
[modify] https://crrev.com/c8848cf49338c354786b72d83a3a2a4ade2f997a/src/codegen/compilation-cache.h

seth.… via monorail

unread,
May 16, 2022, 1:01:30 PM5/16/22
to v8-re...@googlegroups.com

Comment #4 on issue 12808 by seth....@microsoft.com: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c4

Document about options for the next step: https://docs.google.com/document/d/1UksB5Vm7TT1-f3S9W1dK_rP9jKn_ly0WVm_UDPpWuBw/edit?usp=sharing

Git Watcher via monorail

unread,
May 16, 2022, 2:03:07 PM5/16/22
to v8-re...@googlegroups.com

Comment #5 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c5


The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/99a40fa7f75c7264cb9f442993fb64f2892a012f

commit 99a40fa7f75c7264cb9f442993fb64f2892a012f
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Mon May 16 18:01:37 2022

Update test expectations for v8-code-cache.html

Based on my limited understanding, the test case v8-code-cache.html
is a demonstration of bug chromium:1244145, and should emit only PASS
messages once that bug is fixed.

The behavior of this test depends on two separate caches, the isolate
script cache (in memory, in V8) and the serialized code cache (on disk,
managed by Chromium).

The isolate script cache correctly handles host-defined options.
However, those host-defined options are not part of the lookup key for
the isolate script cache's underlying hash table, which is searched
using only the source text. After finding an entry in the table,
CompilationCacheScript::Lookup checks whether the found script has the
correct origin, script-or-module-ness, and host-defined options, and
ignores the result if any of those attributes are incorrect.

The second cache, the serialized code cache, incorrectly handles
host-defined options such as the nonce, which is the topic of bug
chromium:1244145. In the test v8-code-cache.html, any load that misses
the isolate script cache and hits the serialized code cache will
generate an error message.

Before my change https://crrev.com/c/3597106, a script could be added
multiple times to the isolate script cache with different options, but
only the first such instance could be found in a subsequent lookup.
After my change, the isolate script cache can only hold one version of
each script. Attempting to add a second copy of a script with different
options causes a replacement of the first script.

Before my change, all five module loads in this test missed in the
isolate script cache because the first entry for that source text was a
script, not a module. After my change, the second through fourth module
loads hit in the isolate script cache, which changes the fourth load
from a failure to a pass.

Bug: v8:12808, chromium:1244145
Change-Id: I4b9d6332aa381c9e1fa4d4665ea28fcf28ca1ee4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3645098
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
Reviewed-by: Camillo Bruni <cbr...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1003838}

[modify] https://crrev.com/99a40fa7f75c7264cb9f442993fb64f2892a012f/third_party/blink/web_tests/platform/generic/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/v8-code-cache-expected.txt
[modify] https://crrev.com/99a40fa7f75c7264cb9f442993fb64f2892a012f/third_party/blink/web_tests/TestExpectations

Git Watcher via monorail

unread,
May 18, 2022, 11:49:11 AM5/18/22
to v8-re...@googlegroups.com

Comment #6 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c6


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/a76072217a050d2aa8068d93c878ca31eaa6cd23

commit a76072217a050d2aa8068d93c878ca31eaa6cd23
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Tue May 17 21:05:40 2022

Disable recompilation of existing Scripts from Isolate compilation cache

My previous change https://crrev.com/c/3597106 led to some performance
regressions in time spent on parsing and compilation. This change
disables the ability to recompile an existing uncompiled Script, as an
attempt to both fix the regressions and isolate which part of the
previous change was the cause of those problems.

Bug: v8:12808, chromium:1325566, chromium:1325567, chromium:1325601
Change-Id: Ifa086bf27070da8f4b3c0e4415af5ca7b6706b0a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3652252

Reviewed-by: Leszek Swirski <les...@chromium.org>
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>

Git Watcher via monorail

unread,
May 18, 2022, 12:46:12 PM5/18/22
to v8-re...@googlegroups.com

Comment #7 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c7


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/de877f7497026677d4d940e861eac81469bfb7cc

commit de877f7497026677d4d940e861eac81469bfb7cc
Author: Adam Klein <ad...@chromium.org>
Date: Wed May 18 16:44:46 2022

Revert "Disable recompilation of existing Scripts from Isolate compilation cache"

This reverts commit a76072217a050d2aa8068d93c878ca31eaa6cd23.

Reason for revert: fails on GC Stress bot:
https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux%20-%20gc%20stress/38512/overview

Original change's description:

> Disable recompilation of existing Scripts from Isolate compilation cache
>
> My previous change https://crrev.com/c/3597106 led to some performance
> regressions in time spent on parsing and compilation. This change
> disables the ability to recompile an existing uncompiled Script, as an
> attempt to both fix the regressions and isolate which part of the
> previous change was the cause of those problems.
>
> Bug: v8:12808, chromium:1325566, chromium:1325567, chromium:1325601
> Change-Id: Ifa086bf27070da8f4b3c0e4415af5ca7b6706b0a
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3652252
> Reviewed-by: Leszek Swirski <les...@chromium.org>
> Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
> Cr-Commit-Position: refs/heads/main@{#80616}

Bug: v8:12808, chromium:1325566, chromium:1325567, chromium:1325601

Git Watcher via monorail

unread,
May 19, 2022, 8:59:06 AM5/19/22
to v8-re...@googlegroups.com

Comment #8 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c8


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/2df4d58a9e198d8fd7b95f37a01930eb68463d9d

commit 2df4d58a9e198d8fd7b95f37a01930eb68463d9d
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Wed May 18 18:12:13 2022

Fix rehashing of script compilation cache

The script compilation cache contains weak pointers to Script objects as
its keys. When doing a rehashing operation, any hash table needs the
ability to get the hash code for every entry in the table. However, if
the weak pointer was cleared from a key, there is no longer any way to
get the hash code for that entry.

In https://crrev.com/c/3597106 , I attempted to solve this problem by
deleting all entries whose keys contain cleared weak pointers prior to
rehashing, but the implementation has a bug: when resizing, the new
table is allocated after deleting the entries with cleared keys, so if
that allocation triggers a GC, the table can once again have entries
with cleared keys.

This could be solved in a variety of ways, such as:

1. Iterate the entries again and delete those with cleared keys, after
allocating the new table but before calling Rehash() to copy data
into that new table. This means we can't directly use
HashTable::EnsureCapacity, which normally does both the allocation
and the rehashing.
2. Return a bogus hash code for entries whose keys contain cleared weak
pointers. This is simple but risks poor distribution of data after
rehashing.
3. Implement custom rehashing which can avoid copying entries with
cleared keys, rather than reusing the rehashing implementation from
HashTable.
4. Include the hash value in every key, so a consistent hash value is
available even after the weak Script pointer has been cleared.

The fourth option sounds like the lowest risk to me, so this change
implements that option.

Bug: v8:12808
Change-Id: I6b19b9c8af67dcfc31b74842ba581dd141e18845
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3654413

Reviewed-by: Leszek Swirski <les...@chromium.org>
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>

Git Watcher via monorail

unread,
May 19, 2022, 12:51:12 PM5/19/22
to v8-re...@googlegroups.com

Comment #9 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c9


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/16a7150bae6fe9253fd70b32bb2aaa1272cf38ed

commit 16a7150bae6fe9253fd70b32bb2aaa1272cf38ed
Author: Seth Brenith <seth.b...@microsoft.com>

Date: Tue May 17 21:05:40 2022

Reland "Disable recompilation of existing Scripts from Isolate compilation cache"

This is a reland of commit a76072217a050d2aa8068d93c878ca31eaa6cd23

The bug exposed by landing this change the first time has been fixed
separately in https://crrev.com/c/3654413 .


Original change's description:
> Disable recompilation of existing Scripts from Isolate compilation cache
>
> My previous change https://crrev.com/c/3597106 led to some performance
> regressions in time spent on parsing and compilation. This change
> disables the ability to recompile an existing uncompiled Script, as an
> attempt to both fix the regressions and isolate which part of the
> previous change was the cause of those problems.
>
> Bug: v8:12808, chromium:1325566, chromium:1325567, chromium:1325601
> Change-Id: Ifa086bf27070da8f4b3c0e4415af5ca7b6706b0a
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3652252
> Reviewed-by: Leszek Swirski <les...@chromium.org>
> Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
> Cr-Commit-Position: refs/heads/main@{#80616}

Bug: v8:12808, chromium:1325566, chromium:1325567, chromium:1325601
Change-Id: Ib31864bef90ff3340d1dfd4e25e21bef121f2d49
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3655011

Reviewed-by: Leszek Swirski <les...@chromium.org>
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>

Git Watcher via monorail

unread,
May 23, 2022, 10:48:13 AM5/23/22
to v8-re...@googlegroups.com

Comment #10 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c10


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/400b2cc2c64e48360554a0f92f46ac9c599b22a0

commit 400b2cc2c64e48360554a0f92f46ac9c599b22a0
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Fri May 20 16:15:22 2022

Don't rescue old top-level SharedFunctionInfos


My previous change https://crrev.com/c/3597106 led to some performance
regressions in time spent on parsing and compilation. Those regressions
might be due to increasing the reuse of old top-level
SharedFunctionInfos. If the top-level SFI is old enough that its
bytecode can be flushed, then perhaps other SFIs within the script have
already been flushed. In that case, discarding information from a
background compilation or code cache deserialization could be harmful.


Bug: v8:12808, chromium:1325566, chromium:1325567, chromium:1325601
Change-Id: Ia7651bed86eecdbef8878e6132b894ed10163cdc
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3657472
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
Reviewed-by: Leszek Swirski <les...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80697}

[modify] https://crrev.com/400b2cc2c64e48360554a0f92f46ac9c599b22a0/src/objects/compilation-cache-table.cc

Git Watcher via monorail

unread,
May 25, 2022, 5:54:10 AM5/25/22
to v8-re...@googlegroups.com

Comment #11 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c11


The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/03de6a9a16c248a75b4b8637d03da323460653eb

commit 03de6a9a16c248a75b4b8637d03da323460653eb
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Wed May 25 09:53:30 2022

Revert "Update test expectations for v8-code-cache.html"

This reverts commit 99a40fa7f75c7264cb9f442993fb64f2892a012f.

Reason for revert: I've created a reversion CL for the corresponding
V8 code change here: https://crrev.com/c/v8/v8/+/3664023 . This test
expectations change should land first to keep the V8 roller happy.

Original change's description:
> Commit-Queue: Seth Brenith <seth.b...@microsoft.com>

> Reviewed-by: Camillo Bruni <cbr...@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1003838}

Bug: v8:12808, chromium:1244145

Git Watcher via monorail

unread,
May 25, 2022, 8:12:58 AM5/25/22
to v8-re...@googlegroups.com

Comment #12 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c12


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/d21b37d3f27b02c679e0ef233d08c20a6b876ff7

commit d21b37d3f27b02c679e0ef233d08c20a6b876ff7
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Tue May 24 16:30:10 2022

Revert several changes that caused performance regressions

This change reverts the following:

400b2cc2c6 Don't rescue old top-level SharedFunctionInfos
Reviewed on https://chromium-review.googlesource.com/c/v8/v8/+/3657472

16a7150bae Reland "Disable recompilation of existing Scripts from
Isolate compilation cache"
Reviewed on https://chromium-review.googlesource.com/c/v8/v8/+/3655011

2df4d58a9e Fix rehashing of script compilation cache
Reviewed on https://chromium-review.googlesource.com/c/v8/v8/+/3654413

c8848cf493 Refactor CompilationSubCache
Reviewed on https://chromium-review.googlesource.com/c/v8/v8/+/3629603

2507217839 Improve Script reuse in isolate compilation cache, part 1
Reviewed on https://chromium-review.googlesource.com/c/v8/v8/+/3597106

Bug: v8:12808, chromium:1325566, chromium:1325567, chromium:1325601, chromium:1328671, chromium:1328672, chromium:1328678, chromium:1328811, chromium:1328810
Change-Id: I1d318dc172e5214166d3b15f19903186f4fe6024
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3664023
Reviewed-by: Leszek Swirski <les...@chromium.org>
Reviewed-by: Toon Verwaest <verw...@chromium.org>
Commit-Queue: Leszek Swirski <les...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80744}

[modify] https://crrev.com/d21b37d3f27b02c679e0ef233d08c20a6b876ff7/src/objects/hash-table.h
[modify] https://crrev.com/d21b37d3f27b02c679e0ef233d08c20a6b876ff7/src/logging/counters-definitions.h
[modify] https://crrev.com/d21b37d3f27b02c679e0ef233d08c20a6b876ff7/test/cctest/test-serialize.cc
[modify] https://crrev.com/d21b37d3f27b02c679e0ef233d08c20a6b876ff7/test/cctest/test-api.cc
[modify] https://crrev.com/d21b37d3f27b02c679e0ef233d08c20a6b876ff7/src/flags/flag-definitions.h
[modify] https://crrev.com/d21b37d3f27b02c679e0ef233d08c20a6b876ff7/src/objects/hash-table-inl.h
[modify] https://crrev.com/d21b37d3f27b02c679e0ef233d08c20a6b876ff7/src/codegen/compiler.cc
[modify] https://crrev.com/d21b37d3f27b02c679e0ef233d08c20a6b876ff7/src/codegen/compilation-cache.cc
[modify] https://crrev.com/d21b37d3f27b02c679e0ef233d08c20a6b876ff7/src/execution/isolate.h
[modify] https://crrev.com/d21b37d3f27b02c679e0ef233d08c20a6b876ff7/src/objects/compilation-cache-table.h
[modify] https://crrev.com/d21b37d3f27b02c679e0ef233d08c20a6b876ff7/test/cctest/heap/test-heap.cc
[modify] https://crrev.com/d21b37d3f27b02c679e0ef233d08c20a6b876ff7/src/objects/compilation-cache-table.cc
[modify] https://crrev.com/d21b37d3f27b02c679e0ef233d08c20a6b876ff7/src/execution/isolate.cc
[modify] https://crrev.com/d21b37d3f27b02c679e0ef233d08c20a6b876ff7/src/heap/heap.cc
[modify] https://crrev.com/d21b37d3f27b02c679e0ef233d08c20a6b876ff7/src/codegen/compilation-cache.h
[modify] https://crrev.com/d21b37d3f27b02c679e0ef233d08c20a6b876ff7/src/objects/compilation-cache-table-inl.h

Git Watcher via monorail

unread,
May 27, 2022, 9:30:09 AM5/27/22
to v8-re...@googlegroups.com

Comment #13 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c13


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/78a5d6f5a872d1a9f223ff9f4b2ad733787032a4

commit 78a5d6f5a872d1a9f223ff9f4b2ad733787032a4
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Wed May 25 15:26:02 2022

Use InternalIndex-based getters and setters in CompilationCacheTable

This cleanup is expected to have no observable effects.

This is a partial reland of https://crrev.com/c/3597106

Bug: v8:12808
Change-Id: I6b3846f84b804b4a82b2b8601b4c6c93e2779084
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3664015

Reviewed-by: Leszek Swirski <les...@chromium.org>
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>

Git Watcher via monorail

unread,
May 30, 2022, 12:01:08 PM5/30/22
to v8-re...@googlegroups.com

Comment #14 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c14


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/9e2407730fcad0af89263cd0182a7f5b38d2cb61

commit 9e2407730fcad0af89263cd0182a7f5b38d2cb61
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Fri May 27 16:02:56 2022

Reland "Refactor CompilationSubCache"

This is a reland of commit c8848cf49338c354786b72d83a3a2a4ade2f997a

This change was reverted due to a problem in a preceding change. This
relanded version differs in its implementations of the
CompilationCacheScript member functions Lookup, Put, and Age, because
the intent is to not change any behavior.

Original change's description:

> CompilationSubCache has some complexity regarding generations of tables
> which is only used by one subclass, CompilationCacheRegExp. This change
> adjusts the class hierarchy so that classes only contain the necessary
> member functions.
>
> Bug: v8:12808
> Change-Id: I4f4cf15bbf9b80c2de0c18aea82a0c238804759d
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3629603
> Reviewed-by: Leszek Swirski <les...@chromium.org>
> Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
> Cr-Commit-Position: refs/heads/main@{#80506}

Bug: v8:12808
Change-Id: Ib0621b7de8da86a89752c66907f6a56adff9075d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3665936

Reviewed-by: Leszek Swirski <les...@chromium.org>
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>

Git Watcher via monorail

unread,
Jun 1, 2022, 12:17:05 PM6/1/22
to v8-re...@googlegroups.com

Comment #15 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c15


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/4b1b7e29ba66783ff866f21c3439e88672323c66

commit 4b1b7e29ba66783ff866f21c3439e88672323c66
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Fri May 27 16:29:32 2022

Change key format for script cache

This is a partial reland of https://crrev.com/c/3597106 including fixes
from https://crrev.com/c/3654413

Before this change, a script cache key is the same format as an eval
cache key, which is a FixedArray containing:
- The SharedFunctionInfo of the containing function
- The source text
- The language mode in which the code was parsed
- The position in the source where eval was called

After this change, a script cache key is a WeakFixedArray containing:
- A weak pointer to the Script
- The hash value of the source text

This sets up for a subsequent change which can cause these keys to
outlive their corresponding values (top-level SharedFunctionInfos)
without leaking any memory beyond the key itself.

Bug: v8:12808
Change-Id: Ibdfe5d10eafe5b7392e554c500af47975baf45c6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3668304

Reviewed-by: Leszek Swirski <les...@chromium.org>
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>

Git Watcher via monorail

unread,
Jun 2, 2022, 12:40:09 PM6/2/22
to v8-re...@googlegroups.com

Comment #16 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c16


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/c443858fa91d2528c30f2b008d47cfc2e21d4992

commit c443858fa91d2528c30f2b008d47cfc2e21d4992
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Wed Jun 01 21:36:06 2022

Allow lookup of matching scripts in Isolate compilation cache

Currently, if the same script text is compiled multiple times with
differing details (such as name, line number, or host-defined options),
then multiple copies of that script are added to the Isolate's
compilation cache. However, any attempt to look up those scripts can
find only the first instance. This change makes the script compilation
cache behave more consistently by checking the details while searching
the hash table for a match, rather than after a potential match has been
found.

Bug: v8:12808
Change-Id: Ic9da0bf74f359d4f1c88af89d585404f173056ee
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3671615
Reviewed-by: Camillo Bruni <cbr...@chromium.org>
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#80919}

[modify] https://crrev.com/c443858fa91d2528c30f2b008d47cfc2e21d4992/src/objects/compilation-cache-table.cc
[modify] https://crrev.com/c443858fa91d2528c30f2b008d47cfc2e21d4992/test/cctest/test-serialize.cc
[modify] https://crrev.com/c443858fa91d2528c30f2b008d47cfc2e21d4992/src/codegen/compilation-cache.cc
[modify] https://crrev.com/c443858fa91d2528c30f2b008d47cfc2e21d4992/src/objects/compilation-cache-table.h
[modify] https://crrev.com/c443858fa91d2528c30f2b008d47cfc2e21d4992/src/objects/compilation-cache-table-inl.h

Git Watcher via monorail

unread,
Jun 2, 2022, 1:33:05 PM6/2/22
to v8-re...@googlegroups.com

Comment #17 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c17


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/a13598ae0c371ea0e4993a488f8e53db807fc97b

commit a13598ae0c371ea0e4993a488f8e53db807fc97b
Author: Deepti Gandluri <gde...@chromium.org>
Date: Thu Jun 02 17:31:50 2022

Revert "Allow lookup of matching scripts in Isolate compilation cache"

This reverts commit c443858fa91d2528c30f2b008d47cfc2e21d4992.

Reason for revert: Several UBSan failures: https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20UBSan/21547/overview

Original change's description:

> Allow lookup of matching scripts in Isolate compilation cache
>
> Currently, if the same script text is compiled multiple times with
> differing details (such as name, line number, or host-defined options),
> then multiple copies of that script are added to the Isolate's
> compilation cache. However, any attempt to look up those scripts can
> find only the first instance. This change makes the script compilation
> cache behave more consistently by checking the details while searching
> the hash table for a match, rather than after a potential match has been
> found.
>
> Bug: v8:12808
> Change-Id: Ic9da0bf74f359d4f1c88af89d585404f173056ee
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3671615
> Reviewed-by: Camillo Bruni <cbr...@chromium.org>
> Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
> Cr-Commit-Position: refs/heads/main@{#80919}

Bug: v8:12808
Change-Id: I6d007374fb607a2670ca260c6bd0d6774d7f51d7

No-Presubmit: true
No-Tree-Checks: true
No-Try: true

Commit-Queue: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>

Git Watcher via monorail

unread,
Jun 8, 2022, 1:06:05 PM6/8/22
to v8-re...@googlegroups.com

Comment #18 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c18


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/8742d2a273484c01010107f2895f862bb615eb88

commit 8742d2a273484c01010107f2895f862bb615eb88
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Thu Jun 02 17:46:17 2022

Reland "Allow lookup of matching scripts in Isolate compilation cache"

This is a reland of commit c443858fa91d2528c30f2b008d47cfc2e21d4992

The original version included an operation which could left-shift
signed values, which is undefined behavior; the updated version masks
the value first to avoid the problem.


Original change's description:
> Allow lookup of matching scripts in Isolate compilation cache
>
> Currently, if the same script text is compiled multiple times with
> differing details (such as name, line number, or host-defined options),
> then multiple copies of that script are added to the Isolate's
> compilation cache. However, any attempt to look up those scripts can
> find only the first instance. This change makes the script compilation
> cache behave more consistently by checking the details while searching
> the hash table for a match, rather than after a potential match has been
> found.
>
> Bug: v8:12808
> Change-Id: Ic9da0bf74f359d4f1c88af89d585404f173056ee
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3671615
> Reviewed-by: Camillo Bruni <cbr...@chromium.org>
> Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
> Cr-Commit-Position: refs/heads/main@{#80919}

Bug: v8:12808
Change-Id: I494c3c9cc520b79f34247aab6618c40c854b9edc
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3687070

Reviewed-by: Camillo Bruni <cbr...@chromium.org>
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>

Git Watcher via monorail

unread,
Jun 10, 2022, 12:45:04 PM6/10/22
to v8-re...@googlegroups.com

Comment #19 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c19


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/693db0a383e3abfe2df55476128b382a84061c77

commit 693db0a383e3abfe2df55476128b382a84061c77
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Fri Jun 10 15:59:33 2022

Use AddGCPrologueCallback for Isolate compilation cache

Minor refactoring; shouldn't affect behavior.


This is a partial reland of https://crrev.com/c/3597106

Git Watcher via monorail

unread,
Jun 10, 2022, 4:19:04 PM6/10/22
to v8-re...@googlegroups.com

Comment #20 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c20


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/a6d7f261e659dba8e2ec459de2f721ced57efdc8

commit a6d7f261e659dba8e2ec459de2f721ced57efdc8
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Fri Jun 03 15:40:31 2022

Let script compilation cache keys outlive their values


This is a partial reland of https://crrev.com/c/3597106

With this change, an old entry in the script compilation cache is not
completely removed by CompilationCacheScript::Age(). Instead, its value
is replaced with undefined. In that way, the Script is still accessible
from the table until the garbage collector destroys it and clears the
weak pointer.

Bug: v8:12808
Change-Id: Ib494674e67d0fec455e1fed40499c5cca3b7c0a4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3673426
Reviewed-by: Leszek Swirski <les...@chromium.org>
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#81084}

[modify] https://crrev.com/a6d7f261e659dba8e2ec459de2f721ced57efdc8/src/objects/compilation-cache-table.cc
[modify] https://crrev.com/a6d7f261e659dba8e2ec459de2f721ced57efdc8/src/codegen/compilation-cache.cc
[modify] https://crrev.com/a6d7f261e659dba8e2ec459de2f721ced57efdc8/src/objects/compilation-cache-table.h
[modify] https://crrev.com/a6d7f261e659dba8e2ec459de2f721ced57efdc8/src/objects/compilation-cache-table-inl.h

Git Watcher via monorail

unread,
Jun 15, 2022, 12:16:05 PM6/15/22
to v8-re...@googlegroups.com

Comment #21 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c21


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/c5efd19b0e67ea53d39845e18bbbf06677efcd29

commit c5efd19b0e67ea53d39845e18bbbf06677efcd29
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Wed Jun 15 15:27:28 2022

Return both toplevel SFI and Script from compilation cache

This is a partial reland of https://crrev.com/c/3597106 , except for the
changes in compiler.cc, which are just the minimal possible changes to
make the code compile.

With this change, it is possible that a call to
CompilationCache::LookupScript returns any of:
1. A Script and a toplevel SharedFunctionInfo (cache hit)
2. A Script but no toplevel SharedFunctionInfo (partial cache hit)
3. Nothing (cache miss)

Bug: v8:12808
Change-Id: Id33a4cd0cb28562d6b862fbb113ea9d03f255b2b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3687425

Reviewed-by: Leszek Swirski <les...@chromium.org>
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#81193}

[modify] https://crrev.com/c5efd19b0e67ea53d39845e18bbbf06677efcd29/test/cctest/heap/test-heap.cc
[modify] https://crrev.com/c5efd19b0e67ea53d39845e18bbbf06677efcd29/src/objects/compilation-cache-table.cc
[modify] https://crrev.com/c5efd19b0e67ea53d39845e18bbbf06677efcd29/test/cctest/test-serialize.cc
[modify] https://crrev.com/c5efd19b0e67ea53d39845e18bbbf06677efcd29/src/logging/counters-definitions.h
[modify] https://crrev.com/c5efd19b0e67ea53d39845e18bbbf06677efcd29/src/codegen/compiler.cc
[modify] https://crrev.com/c5efd19b0e67ea53d39845e18bbbf06677efcd29/src/codegen/compilation-cache.cc
[modify] https://crrev.com/c5efd19b0e67ea53d39845e18bbbf06677efcd29/src/codegen/compilation-cache.h
[modify] https://crrev.com/c5efd19b0e67ea53d39845e18bbbf06677efcd29/src/objects/compilation-cache-table.h

jianx… via monorail

unread,
Jun 22, 2022, 4:34:44 AM6/22/22
to v8-re...@googlegroups.com

Comment #22 on issue 12808 by jianx...@intel.com: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c22

Hello,

I've come across a similar scenario where compilation still takes a long time when reloading the page.

It can be solved with flags "no_isolate_script_cache_ageing" and "no-flush-bytecode".

I've noticed your patch[1], seems that it tried to reserve some cache even it is already old, can it be considered as a "smarter" implementation for " no_isolate_script_cache_ageing"?

[1] https://chromium-review.googlesource.com/c/v8/v8/+/3597106

seth.… via monorail

unread,
Jun 23, 2022, 1:46:53 PM6/23/22
to v8-re...@googlegroups.com

Comment #23 on issue 12808 by seth....@microsoft.com: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c23

Correct. Once this issue is complete, the flag no_isolate_script_cache_ageing should have no effect on performance. However, bytecode flushing will still be enabled, so if your scenario requires no-flush-bytecode, you won't see any benefit.

Git Watcher via monorail

unread,
Jun 23, 2022, 2:40:07 PM6/23/22
to v8-re...@googlegroups.com

Comment #24 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c24


The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/eaa564deae06081df5c7d27d71281d5b61476e55

commit eaa564deae06081df5c7d27d71281d5b61476e55
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Thu Jun 23 18:39:38 2022

Reland "Update test expectations for v8-code-cache.html"

This is a reland of commit 99a40fa7f75c7264cb9f442993fb64f2892a012f

After https://chromium-review.googlesource.com/c/v8/v8/+/3687070 , the
V8 compilation cache can now reliably find scripts that have matching
source text but mismatched options.
Change-Id: I36b03a9e79a36797ab52f1570aebcd84c087d908
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3709968

Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
Reviewed-by: Camillo Bruni <cbr...@chromium.org>

jianx… via monorail

unread,
Jun 23, 2022, 9:34:57 PM6/23/22
to v8-re...@googlegroups.com

Comment #25 on issue 12808 by jianx...@intel.com: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c25

Great! Thanks for the explanation! I am trying to implement the adaptive bytecode flushing (ageing) [1], to set the old age dynamically as a smarter "no-flush-bytecode". Ideally, my scenario with benefit from that.


[1] https://chromium-review.googlesource.com/c/v8/v8/+/3678486

Git Watcher via monorail

unread,
Jun 28, 2022, 12:19:04 PM6/28/22
to v8-re...@googlegroups.com

Comment #26 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c26


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/80575e2816036567c2c8a52ea11f236f946ea940

commit 80575e2816036567c2c8a52ea11f236f946ea940
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Tue Jun 28 15:18:05 2022

Allow embedder to provide source text during code cache deserialization

This change is only to get the API in place; the newly added functions
don't yet do anything.

Bug: v8:12808
Change-Id: Ic6a697d4f62c2b61761b2545dae6fcdf37653bbf
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3681880

Reviewed-by: Leszek Swirski <les...@chromium.org>
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>

Git Watcher via monorail

unread,
Jun 30, 2022, 11:42:06 AM6/30/22
to v8-re...@googlegroups.com

Comment #27 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c27


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/1f97a2dfcb06c9b5207125646a42b4110909594f

commit 1f97a2dfcb06c9b5207125646a42b4110909594f
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Wed Jun 29 16:43:42 2022

Reuse existing Scripts during synchronous parsing


This is a partial reland of https://crrev.com/c/3597106

With this change, an existing Script from the compilation cache can be
reused after its top-level SharedFunctionInfo was discarded, but only if
the new script is parsed on the main thread (not deserialized from code
cache data, and not parsed on a background thread).

Bug: v8:12808
Change-Id: I1edaee2095306a89e2c3b91f2fd01ac053f3c770
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3689348

Reviewed-by: Leszek Swirski <les...@chromium.org>
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>

Git Watcher via monorail

unread,
Jul 25, 2022, 1:30:05 PM7/25/22
to v8-re...@googlegroups.com

Comment #28 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c28


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/e895b7af73728b0f1431549dbd52db37e8b1b577

commit e895b7af73728b0f1431549dbd52db37e8b1b577
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Mon Jul 25 15:39:46 2022

Background merging of deserialized scripts

Recently, https://crrev.com/c/v8/v8/+/3681880 added new API functions
with which an embedder could request that V8 merge newly deserialized
script data into an existing Script from the Isolate's compilation
cache. This change implements those new functions. This functionality is
still disabled by default due to the flag
merge_background_deserialized_script_with_compilation_cache.

The goal of this new functionality is to reduce memory usage when
multiple frames load the same script with a long delay between (long
enough for the script to have been evicted from Blink's in-memory cache
and for the top-level SharedFunctionInfo to be flushed). In that case,
there are two Script objects for the same script: one which was found in
the Isolate compilation cache (the "old" script), and one which was
recently deserialized (the "new" script). The new script's object graph
is essentially standalone: it may point to internalized strings and
readonly objects such as the empty feedback metadata, but otherwise
it is unconnected to the rest of the heap. The merging logic takes any
useful data from the new script's object graph and attaches it into the
old script's object graph, so that the new Script object and any other
duplicated objects can be discarded. More specifically:

1. If the new Script has a SharedFunctionInfo for a particular function
literal, and the old Script does not, then the old Script is updated
to refer to the new SharedFunctionInfo.
2. If the new Script has a compiled SharedFunctionInfo for a particular
function literal, and the old Script has an uncompiled
SharedFunctionInfo, then the old SharedFunctionInfo is updated to
point to the function_data and feedback_metadata from the new
SharedFunctionInfo.
3. If any used object from the new object graph points to a
SharedFunctionInfo, where the old object graph contains a matching
SharedFunctionInfo for the same function literal, then that pointer
is updated to point to the old SharedFunctionInfo.

The document at [0] includes diagrams showing an example merge on a very
small script.

Steps 1 and 2 above are pretty simple, but step 3 requires walking a
possibly large set of objects, so this new API lets the embedder run
step 3 from a background thread. Steps 1 and 2 are performed later, on
the main thread.

The next important question is: in what ways can the old script's object
graph be modified during the background execution of step 3, or during
the time after step 3 but before steps 1 and 2?

A. SharedFunctionInfos can go from compiled to uncompiled due to
flushing. This is okay; the worst outcome is that the function would
need to be compiled again later. Such a risk is already present,
since V8 doesn't keep IsCompiledScopes for every compiled function in
a background-deserialized script.
B. SharedFunctionInfos can go from uncompiled to compiled due to lazy
compilation. This is also okay; the merge completion logic on the
main thread will just keep this lazily compiled data rather than
inserting compiled data from the newly deserialized object graph.
C. SharedFunctionInfos can be cleared from the Script's weak array if
they are no longer referenced. This is mostly okay, because any
SharedFunctionInfo that is needed by the background merge is strongly
referenced and therefore can't be cleared. The only problem arises if
the top-level SharedFunctionInfo gets cleared, so the merge task must
deliberately keep a reference to that one.
D. SharedFunctionInfos can be created if they are needed due to lazy
compilation of a parent function. This change is somewhat troublesome
because it invalidates the background thread's work and requires a
re-traversal on the main thread to update any pointers that should
point to this lazily compiled SharedFunctionInfo.

At a high level, this change implements three previously unimplemented
functions in BackgroundDeserializeTask (in compiler.cc) and updates one:

- BackgroundDeserializeTask::SourceTextAvailable, run on the main
thread, checks whether there is a matching Script in the Isolate
compilation cache which doesn't already have a top-level
SharedFunctionInfo. If so, it saves that Script in a persistent
handle.
- BackgroundDeserializeTask::ShouldMergeWithExistingScript checks
whether the persistent handle from the first step exists (a fast
operation which can be called from any thread).
- BackgroundDeserializeTask::MergeWithExistingScript, run on a
background thread, performs step 3 of the merge described above and
generates lists of persistent data describing how the main thread can
complete the merge.
- BackgroundDeserializeTask::Finish is updated to perform the merge
steps 1 and 2 listed above, as well as a possible re-traversal of the
graph if required due to newly created SharedFunctionInfos in the old
Script.

The merge logic has nothing to do with deserialization, and indeed I
hope to reuse it for background compilation tasks as well, so it is all
contained within a new class BackgroundMergeTask (in compiler.h,cc). It
uses a second class, ForwardPointersVisitor (in compiler.cc) to perform
the object visitation that updates pointers to SharedFunctionInfos.

[0] https://docs.google.com/document/d/1UksB5Vm7TT1-f3S9W1dK_rP9jKn_ly0WVm_UDPpWuBw/edit

Bug: v8:12808
Change-Id: Id405869e9d5b106ca7afd9c4b08cb5813e6852c6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3739232

Reviewed-by: Leszek Swirski <les...@chromium.org>
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#81941}

[modify] https://crrev.com/e895b7af73728b0f1431549dbd52db37e8b1b577/test/unittests/api/deserialize-unittest.cc
[modify] https://crrev.com/e895b7af73728b0f1431549dbd52db37e8b1b577/src/objects/compilation-cache-table.cc
[add] https://crrev.com/e895b7af73728b0f1431549dbd52db37e8b1b577/src/codegen/background-merge-task.h
[modify] https://crrev.com/e895b7af73728b0f1431549dbd52db37e8b1b577/src/snapshot/code-serializer.h
[modify] https://crrev.com/e895b7af73728b0f1431549dbd52db37e8b1b577/src/codegen/compiler.h
[modify] https://crrev.com/e895b7af73728b0f1431549dbd52db37e8b1b577/src/codegen/compiler.cc
[modify] https://crrev.com/e895b7af73728b0f1431549dbd52db37e8b1b577/BUILD.gn
[modify] https://crrev.com/e895b7af73728b0f1431549dbd52db37e8b1b577/src/snapshot/code-serializer.cc
[modify] https://crrev.com/e895b7af73728b0f1431549dbd52db37e8b1b577/BUILD.bazel
[modify] https://crrev.com/e895b7af73728b0f1431549dbd52db37e8b1b577/src/objects/objects.cc
[modify] https://crrev.com/e895b7af73728b0f1431549dbd52db37e8b1b577/src/objects/fixed-array.h

Git Watcher via monorail

unread,
Jul 25, 2022, 7:02:05 PM7/25/22
to v8-re...@googlegroups.com

Comment #29 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c29


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/44fc1fdac2c1f6dea3e1670dfa16c6cba9c78d14

commit 44fc1fdac2c1f6dea3e1670dfa16c6cba9c78d14
Author: Deepti Gandluri <gde...@chromium.org>
Date: Mon Jul 25 23:00:17 2022

Revert "Background merging of deserialized scripts"

This reverts commit e895b7af73728b0f1431549dbd52db37e8b1b577.

Reason for revert: TSAN failures: https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20TSAN%20-%20stress-incremental-marking/8468/overview

Original change's description:
Bug: v8:12808
Change-Id: I82a080e6287828445293cb6b4b94a5e8f15eb8f3

No-Presubmit: true
No-Tree-Checks: true
No-Try: true

Auto-Submit: Deepti Gandluri <gde...@chromium.org>
Commit-Queue: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>
Owners-Override: Deepti Gandluri <gde...@chromium.org>
Bot-Commit: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#81943}

[modify] https://crrev.com/44fc1fdac2c1f6dea3e1670dfa16c6cba9c78d14/test/unittests/api/deserialize-unittest.cc
[modify] https://crrev.com/44fc1fdac2c1f6dea3e1670dfa16c6cba9c78d14/src/objects/compilation-cache-table.cc
[delete] https://crrev.com/f52762254617e57bc36ac332191423a4b1ae4732/src/codegen/background-merge-task.h
[modify] https://crrev.com/44fc1fdac2c1f6dea3e1670dfa16c6cba9c78d14/src/codegen/compiler.cc
[modify] https://crrev.com/44fc1fdac2c1f6dea3e1670dfa16c6cba9c78d14/src/snapshot/code-serializer.h
[modify] https://crrev.com/44fc1fdac2c1f6dea3e1670dfa16c6cba9c78d14/src/codegen/compiler.h
[modify] https://crrev.com/44fc1fdac2c1f6dea3e1670dfa16c6cba9c78d14/BUILD.gn
[modify] https://crrev.com/44fc1fdac2c1f6dea3e1670dfa16c6cba9c78d14/src/snapshot/code-serializer.cc
[modify] https://crrev.com/44fc1fdac2c1f6dea3e1670dfa16c6cba9c78d14/BUILD.bazel
[modify] https://crrev.com/44fc1fdac2c1f6dea3e1670dfa16c6cba9c78d14/src/objects/objects.cc
[modify] https://crrev.com/44fc1fdac2c1f6dea3e1670dfa16c6cba9c78d14/src/objects/fixed-array.h

Git Watcher via monorail

unread,
Jul 28, 2022, 1:03:07 PM7/28/22
to v8-re...@googlegroups.com

Comment #30 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c30


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/766b2a4d52d7b768ad504a352e711269c26b6d93

commit 766b2a4d52d7b768ad504a352e711269c26b6d93
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Thu Jul 28 15:56:42 2022

Reland "Background merging of deserialized scripts"

This is a reland of commit e895b7af73728b0f1431549dbd52db37e8b1b577

The unit test has been updated to work correctly when
--stress-incremental-marking is enabled.
Change-Id: Id2036dfa4eba8670cac899773d7a906825fa2c50
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3787266

Reviewed-by: Leszek Swirski <les...@chromium.org>
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#82045}

[modify] https://crrev.com/766b2a4d52d7b768ad504a352e711269c26b6d93/test/unittests/api/deserialize-unittest.cc
[modify] https://crrev.com/766b2a4d52d7b768ad504a352e711269c26b6d93/src/objects/compilation-cache-table.cc
[add] https://crrev.com/766b2a4d52d7b768ad504a352e711269c26b6d93/src/codegen/background-merge-task.h
[modify] https://crrev.com/766b2a4d52d7b768ad504a352e711269c26b6d93/src/snapshot/code-serializer.h
[modify] https://crrev.com/766b2a4d52d7b768ad504a352e711269c26b6d93/src/codegen/compiler.h
[modify] https://crrev.com/766b2a4d52d7b768ad504a352e711269c26b6d93/src/codegen/compiler.cc
[modify] https://crrev.com/766b2a4d52d7b768ad504a352e711269c26b6d93/BUILD.gn
[modify] https://crrev.com/766b2a4d52d7b768ad504a352e711269c26b6d93/src/snapshot/code-serializer.cc
[modify] https://crrev.com/766b2a4d52d7b768ad504a352e711269c26b6d93/BUILD.bazel
[modify] https://crrev.com/766b2a4d52d7b768ad504a352e711269c26b6d93/src/objects/objects.cc
[modify] https://crrev.com/766b2a4d52d7b768ad504a352e711269c26b6d93/src/objects/fixed-array.h

Git Watcher via monorail

unread,
Aug 1, 2022, 6:43:11 PM8/1/22
to v8-re...@googlegroups.com

Comment #31 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c31


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/a1392fa113a7e85c09e6066ce77686c52087f21b

commit a1392fa113a7e85c09e6066ce77686c52087f21b
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Mon Aug 01 21:57:39 2022

Enable background merging when --stress-background-compile

This change adds new functions to BackgroundCompileTask which closely
match those in BackgroundDeserializeTask. These functions allow a caller
to manage background merging of newly compiled content into an existing
Script from the Isolate compilation cache. These functions are not yet
exposed via the API; instead, StressBackgroundCompileThread uses them to
increase test coverage of the merging logic.

Bug: v8:12808
Change-Id: I4d2f429164223785169fe447ce2bdd8beaee00d4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3793959
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
Reviewed-by: Leszek Swirski <les...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82121}

[modify] https://crrev.com/a1392fa113a7e85c09e6066ce77686c52087f21b/test/cctest/heap/test-heap.cc
[modify] https://crrev.com/a1392fa113a7e85c09e6066ce77686c52087f21b/src/codegen/background-merge-task.h
[modify] https://crrev.com/a1392fa113a7e85c09e6066ce77686c52087f21b/src/codegen/compiler.cc
[modify] https://crrev.com/a1392fa113a7e85c09e6066ce77686c52087f21b/src/codegen/compiler.h

Git Watcher via monorail

unread,
Aug 2, 2022, 7:52:11 AM8/2/22
to v8-re...@googlegroups.com

Comment #32 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c32


The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/7fbc16f65a0d82e87ec0e7c967312814e253cfe3

commit 7fbc16f65a0d82e87ec0e7c967312814e253cfe3
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Tue Aug 02 11:51:03 2022

Provide source text to V8 during code cache consumption

In https://crrev.com/c/v8/v8/+/3681880 , V8 added several new API
functions for merging newly deserialized code into an existing Script
from the Isolate's compilation cache. Those new API functions are still
disabled by default, but once enabled, they will allow for reducing
memory usage in cases where multiple frames load the same scripts with a
long delay between (long enough for the garbage collector to discard
data from the initial script evaluation).

This change updates Blink to call those new API functions. Three logic
changes are necessary in script_cache_consumer.cc:

1. Once the full source text is available, provide it to V8 via the
function SourceTextAvailable.
2. Once the deserialization task has completed and the source text has
been provided to V8, check whether further background work is
needed by calling ShouldMergeWithExistingScript.
3. If ShouldMergeWithExistingScript returns true, run
MergeWithExistingScript on a background thread.

The rest of this change is related to providing the data needed in step
1 above. The V8 API function SourceTextAvailable requires both the
source text string and the ScriptOrigin. Usually, ScriptOrigin is
produced during V8ScriptRunner::CompileScript, which refers to a
ClassicScript. However, in this new code, there isn't yet a
ClassicScript; we must instead produce the ScriptOrigin based on a
ClassicPendingScript.

Bug: v8:12808
Change-Id: I8e4cfebf8ca0eab0f98d1cf0ec086d4c70d7dcb8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3682122
Reviewed-by: Kouhei Ueno <kou...@chromium.org>
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#1030492}

[modify] https://crrev.com/7fbc16f65a0d82e87ec0e7c967312814e253cfe3/third_party/blink/renderer/bindings/core/v8/script_cache_consumer.cc
[modify] https://crrev.com/7fbc16f65a0d82e87ec0e7c967312814e253cfe3/third_party/blink/renderer/bindings/core/v8/script_cache_consumer_client.h
[modify] https://crrev.com/7fbc16f65a0d82e87ec0e7c967312814e253cfe3/third_party/blink/renderer/bindings/core/v8/script_cache_consumer.h
[modify] https://crrev.com/7fbc16f65a0d82e87ec0e7c967312814e253cfe3/third_party/blink/renderer/bindings/core/v8/v8_script_runner_test.cc
[modify] https://crrev.com/7fbc16f65a0d82e87ec0e7c967312814e253cfe3/third_party/blink/renderer/core/script/classic_pending_script.cc
[modify] https://crrev.com/7fbc16f65a0d82e87ec0e7c967312814e253cfe3/third_party/blink/renderer/core/script/classic_script.h
[modify] https://crrev.com/7fbc16f65a0d82e87ec0e7c967312814e253cfe3/third_party/blink/renderer/core/script/classic_script.cc
[modify] https://crrev.com/7fbc16f65a0d82e87ec0e7c967312814e253cfe3/third_party/blink/renderer/core/script/classic_pending_script.h

Git Watcher via monorail

unread,
Aug 2, 2022, 8:47:15 AM8/2/22
to v8-re...@googlegroups.com

Comment #33 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c33


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/0669c5bf9ca8876fba742c6e2a0c12360d36139a

commit 0669c5bf9ca8876fba742c6e2a0c12360d36139a
Author: Nico Hartmann <nicoha...@chromium.org>
Date: Tue Aug 02 12:44:57 2022

Revert "Enable background merging when --stress-background-compile"

This reverts commit a1392fa113a7e85c09e6066ce77686c52087f21b.

Reason for revert: https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20GC%20Stress%20-%20custom%20snapshot/43149/overview

Original change's description:

> Enable background merging when --stress-background-compile
>
> This change adds new functions to BackgroundCompileTask which closely
> match those in BackgroundDeserializeTask. These functions allow a caller
> to manage background merging of newly compiled content into an existing
> Script from the Isolate compilation cache. These functions are not yet
> exposed via the API; instead, StressBackgroundCompileThread uses them to
> increase test coverage of the merging logic.
>
> Bug: v8:12808
> Change-Id: I4d2f429164223785169fe447ce2bdd8beaee00d4
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3793959
> Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
> Reviewed-by: Leszek Swirski <les...@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#82121}

Bug: v8:12808
Change-Id: Ibb0bc2adb79e4655b39a8a6ac33d8c8ffc5ebdb9

No-Presubmit: true
No-Tree-Checks: true
No-Try: true

Git Watcher via monorail

unread,
Aug 3, 2022, 8:16:12 AM8/3/22
to v8-re...@googlegroups.com

Comment #34 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c34


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/13ecd2c37426b7a2347055a77d79d45d28f6a366

commit 13ecd2c37426b7a2347055a77d79d45d28f6a366
Author: Seth Brenith <seth.b...@microsoft.com>

Date: Mon Aug 01 21:57:39 2022

Reland "Enable background merging when --stress-background-compile"

This is a reland of commit a1392fa113a7e85c09e6066ce77686c52087f21b

The original change was reverted due to v8:13135, which was fixed
separately.


Original change's description:
> Enable background merging when --stress-background-compile
>
> This change adds new functions to BackgroundCompileTask which closely
> match those in BackgroundDeserializeTask. These functions allow a caller
> to manage background merging of newly compiled content into an existing
> Script from the Isolate compilation cache. These functions are not yet
> exposed via the API; instead, StressBackgroundCompileThread uses them to
> increase test coverage of the merging logic.
>
> Bug: v8:12808
> Change-Id: I4d2f429164223785169fe447ce2bdd8beaee00d4
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3793959
> Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
> Reviewed-by: Leszek Swirski <les...@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#82121}

Bug: v8:12808
Change-Id: I530c6e87bbad11e178ef4abfdc25fa98530f0224

Git Watcher via monorail

unread,
Aug 17, 2022, 4:34:08 PM8/17/22
to v8-re...@googlegroups.com

Comment #35 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c35


The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/edba40c7f897a39306dfa3ae151ed8134b455b46

commit edba40c7f897a39306dfa3ae151ed8134b455b46
Author: Hiroshige Hayashizaki <hiro...@chromium.org>
Date: Wed Aug 17 20:33:41 2022

Create ClassicScript at ClassicPendingScript::NotifyFinished()

To make ClassicScript accessible from ScriptCacheConsumer
and thus avoid accessing duplicated information
via ScriptCacheConsumerClient.
A follow-up of
https://chromium-review.googlesource.com/c/chromium/src/+/3682122.

Bug: v8:12808, 1349934
Change-Id: I22c9c3e11998a3b88b61639d857d54120a5bfcdd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3808872
Reviewed-by: Shunya Shishido <sisid...@chromium.org>
Reviewed-by: Kouhei Ueno <kou...@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiro...@chromium.org>
Reviewed-by: Minoru Chikamune <chik...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1036260}

[modify] https://crrev.com/edba40c7f897a39306dfa3ae151ed8134b455b46/third_party/blink/renderer/bindings/core/v8/script_cache_consumer.cc
[modify] https://crrev.com/edba40c7f897a39306dfa3ae151ed8134b455b46/third_party/blink/renderer/bindings/core/v8/script_cache_consumer_client.h
[modify] https://crrev.com/edba40c7f897a39306dfa3ae151ed8134b455b46/third_party/blink/renderer/bindings/core/v8/script_cache_consumer.h
[modify] https://crrev.com/edba40c7f897a39306dfa3ae151ed8134b455b46/third_party/blink/renderer/core/script/classic_pending_script.cc
[modify] https://crrev.com/edba40c7f897a39306dfa3ae151ed8134b455b46/third_party/blink/renderer/bindings/core/v8/v8_script_runner_test.cc
[modify] https://crrev.com/edba40c7f897a39306dfa3ae151ed8134b455b46/third_party/blink/renderer/core/script/classic_script.h
[modify] https://crrev.com/edba40c7f897a39306dfa3ae151ed8134b455b46/third_party/blink/renderer/bindings/core/v8/script_streamer_test.cc
[modify] https://crrev.com/edba40c7f897a39306dfa3ae151ed8134b455b46/third_party/blink/renderer/core/script/classic_script.cc
[modify] https://crrev.com/edba40c7f897a39306dfa3ae151ed8134b455b46/third_party/blink/renderer/core/script/classic_pending_script.h

Git Watcher via monorail

unread,
Aug 23, 2022, 12:28:08 PM8/23/22
to v8-re...@googlegroups.com

Comment #36 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c36


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/0f1fc1e0ef5aa39d59a15b7036d35faba3124e4f

commit 0f1fc1e0ef5aa39d59a15b7036d35faba3124e4f
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Tue Aug 23 14:36:00 2022

Enable merging deserialized scripts when --future is set

This can save memory in cases where multiple frames use the same script,
with sufficient time between loads that the script's top-level
SharedFunctionInfo is no longer present in the compilation cache.
Merging is relatively fast; it generally takes about one tenth as long
as deserialization.


Bug: v8:12808
Change-Id: I317a89b77fb218798dfc9dfd888e808b17d62fdd
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3845792

Reviewed-by: Leszek Swirski <les...@chromium.org>
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>

Git Watcher via monorail

unread,
Sep 1, 2022, 6:47:06 PM9/1/22
to v8-re...@googlegroups.com

Comment #37 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c37


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/f388f96fdbf53b159229bca930f9561d57c1e030

commit f388f96fdbf53b159229bca930f9561d57c1e030
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Wed Aug 31 20:18:17 2022

Enable background merging of deserialized scripts

This can save memory in cases where multiple frames in a process use the

same script, with sufficient time between loads that the script's
top-level SharedFunctionInfo is no longer present in the compilation
cache. Merging is relatively fast; it generally takes about one tenth as
long as deserialization.

Bug: v8:12808
Change-Id: I7366a51f1d2ca6a9f551cdf2bdbe0441450cf1bb
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3868088

Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
Reviewed-by: Leszek Swirski <les...@chromium.org>

Git Watcher via monorail

unread,
Sep 5, 2022, 10:27:11 AM9/5/22
to v8-re...@googlegroups.com

Comment #38 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c38


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/9dedaaf3135af9f0620d37c96cc6aee82016fdd7

commit 9dedaaf3135af9f0620d37c96cc6aee82016fdd7
Author: Leszek Swirski <les...@chromium.org>
Date: Mon Sep 05 13:44:59 2022

Revert "Enable background merging of deserialized scripts"

This reverts commit f388f96fdbf53b159229bca930f9561d57c1e030.

Reason for revert: Crashes in canary (https://crbug.com/1360024)

Original change's description:

> Enable background merging of deserialized scripts
>
> This can save memory in cases where multiple frames in a process use the
> same script, with sufficient time between loads that the script's
> top-level SharedFunctionInfo is no longer present in the compilation
> cache. Merging is relatively fast; it generally takes about one tenth as
> long as deserialization.
>
> Bug: v8:12808
> Change-Id: I7366a51f1d2ca6a9f551cdf2bdbe0441450cf1bb
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3868088
> Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
> Reviewed-by: Leszek Swirski <les...@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#82920}

Bug: v8:12808
Change-Id: If160173afaab351d995ddcf4b60d6efe656cf70b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3871208
Bot-Commit: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>
Auto-Submit: Leszek Swirski <les...@chromium.org>
Commit-Queue: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#82983}

[modify] https://crrev.com/9dedaaf3135af9f0620d37c96cc6aee82016fdd7/src/flags/flag-definitions.h

Git Watcher via monorail

unread,
Sep 7, 2022, 12:03:06 PM9/7/22
to v8-re...@googlegroups.com
Updates:
Labels: merge-merged-5284

Comment #39 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c39


The following revision refers to this bug:

Author: Leszek Swirski <les...@chromium.org>
Date: Mon Sep 05 13:44:59 2022

[Merge to Android Dev branch 5284] Revert "Enable background merging of deserialized scripts"


This reverts commit f388f96fdbf53b159229bca930f9561d57c1e030.

Reason for revert: Crashes in canary (https://crbug.com/1360024)

Original change's description:
> Enable background merging of deserialized scripts
>
> This can save memory in cases where multiple frames in a process use the
> same script, with sufficient time between loads that the script's
> top-level SharedFunctionInfo is no longer present in the compilation
> cache. Merging is relatively fast; it generally takes about one tenth as
> long as deserialization.
>
> Bug: v8:12808
> Change-Id: I7366a51f1d2ca6a9f551cdf2bdbe0441450cf1bb
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3868088
> Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
> Reviewed-by: Leszek Swirski <les...@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#82920}

Bug: v8:12808
Change-Id: If160173afaab351d995ddcf4b60d6efe656cf70b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3871208
Bot-Commit: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>
Auto-Submit: Leszek Swirski <les...@chromium.org>
Commit-Queue: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#82983}
(cherry picked from commit 9dedaaf3135af9f0620d37c96cc6aee82016fdd7)
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3878766
Commit-Queue: Leszek Swirski <les...@chromium.org>
Reviewed-by: Leszek Swirski <les...@chromium.org>

[modify] https://crrev.com/052a27555809a246cc6a0531e1a90e6ec991b951/src/flags/flag-definitions.h

Git Watcher via monorail

unread,
Sep 12, 2022, 8:45:12 AM9/12/22
to v8-re...@googlegroups.com

Comment #40 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c40


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/c8d1ca8a2c445d4031852398c50c2d1b095727f1

commit c8d1ca8a2c445d4031852398c50c2d1b095727f1
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Fri Sep 09 17:53:47 2022

Fix crash in background merging of deserialized scripts

BackgroundMergeTask::CompleteMergeInForeground contained an incorrect
assumption that some SharedFunctionInfos would have bytecode arrays.

Bug: v8:12808, chromium:1360024
Change-Id: I42ca22fc3a4412aea5e5a433e63c685eaf2af242
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3888198

Reviewed-by: Leszek Swirski <les...@chromium.org>
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>

Git Watcher via monorail

unread,
Sep 26, 2022, 1:04:07 PM9/26/22
to v8-re...@googlegroups.com

Comment #41 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c41


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/890ee74ca70e20d38f33e740946b9d0701c13346

commit 890ee74ca70e20d38f33e740946b9d0701c13346
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Fri Sep 23 12:33:46 2022

Allow no-op background merges to complete

It is possible, though unlikely, that V8 will deserialize code cache
data, decide to merge that new data with an existing script from the
Isolate compilation cache, and subsequently do nothing in the background
portion of the merge (make no heap changes, and request no follow-up
changes on the main thread). In this case, the most optimal outcome is
to reuse the script from the Isolate compilation cache, not to use the
newly deserialized script.

CodeSerializer::FinishOffThreadDeserialize uses
BackgroundMergeTask::HasPendingForegroundWork to determine whether it
should complete the merge and use the Script from the compilation cache
or complete the deserialization and use the newly deserialized Script.
This change updates HasPendingForegroundWork so that it will return true
even if the merge was a no-op.

Bug: v8:12808
Change-Id: I08fcb814e797218e5be2b4ce4f45bd4e0637ec80
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3916270

Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
Reviewed-by: Leszek Swirski <les...@chromium.org>

Git Watcher via monorail

unread,
Oct 20, 2022, 11:49:08 AM10/20/22
to v8-re...@googlegroups.com

Comment #42 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c42


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/d1dcdd9a21948824729b7c7cb96fb2c0cc633764

commit d1dcdd9a21948824729b7c7cb96fb2c0cc633764
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Thu Oct 20 13:11:22 2022

Also copy scope info and flags when merging deserialized script

When merging a newly deserialized script into an existing one from the
compilation cache, it is often the case that a SharedFunctionInfo from
the cached script ("old SFI") has been flushed and the corresponding
SharedFunctionInfo from the new script ("new SFI") is compiled. In that
case, it is sufficient to copy the bytecode array and feedback metadata
from the new SFI to the old SFI, as already implemented.

However, there is another case to consider: perhaps the new SFI is
compiled and the old SFI was never compiled. In that case, the old SFI
has no ScopeInfo and some of its flags may be incorrect.

To fix the problem, this CL updates CompleteMergeInForeground to copy
everything except script_or_debug_info from the new SFI to the old SFI.

This change implies some duplication of ScopeInfos, since each ScopeInfo
can point to its parent, so matching parent ScopeInfos from the new and
old scripts will coexist. However, this isn't a new problem: similar
duplication is already caused by the portion of the merge algorithm
which attaches new compiled SFIs into the old Script where the old
Script doesn't have a matching SFI. I don't see any way in which this
duplication would cause incorrect behavior. In fact, it is possible to
get duplicated ScopeInfos without any merging at all, which indicates to
me that such duplication is safe. Duplication occurs if a SFI is flushed
or removed while one of its descendant functions is still alive, and
subsequently the same function literal is compiled again.

Bug: v8:12808, chromium:1359773
Change-Id: I2a3a720021c797c62a87d10e999603ff5e29a027
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3965723

Reviewed-by: Leszek Swirski <les...@chromium.org>
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>

Git Watcher via monorail

unread,
Oct 24, 2022, 9:57:10 AM10/24/22
to v8-re...@googlegroups.com

Comment #43 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c43


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/251512981d0c09c4cfe8890f68bd5d96f96bd704

commit 251512981d0c09c4cfe8890f68bd5d96f96bd704
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Fri Oct 21 17:45:34 2022

Reland "Enable background merging of deserialized scripts"

This reverts commit 9dedaaf3135af9f0620d37c96cc6aee82016fdd7.

Reason for revert: crashes have been fixed:
- https://crrev.com/c/v8/v8/+/3888198
- https://crrev.com/c/v8/v8/+/3965723

Original change's description:

> Revert "Enable background merging of deserialized scripts"
>
> This reverts commit f388f96fdbf53b159229bca930f9561d57c1e030.
>
> Reason for revert: Crashes in canary (https://crbug.com/1360024)
>
> Original change's description:
> > Enable background merging of deserialized scripts
> >
> > This can save memory in cases where multiple frames in a process use the
> > same script, with sufficient time between loads that the script's
> > top-level SharedFunctionInfo is no longer present in the compilation
> > cache. Merging is relatively fast; it generally takes about one tenth as
> > long as deserialization.
> >
> > Bug: v8:12808
> > Change-Id: I7366a51f1d2ca6a9f551cdf2bdbe0441450cf1bb
> > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3868088
> > Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
> > Reviewed-by: Leszek Swirski <les...@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#82920}
>
> Bug: v8:12808
> Change-Id: If160173afaab351d995ddcf4b60d6efe656cf70b
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3871208
> Bot-Commit: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>
> Auto-Submit: Leszek Swirski <les...@chromium.org>
> Commit-Queue: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>
> Cr-Commit-Position: refs/heads/main@{#82983}

Bug: v8:12808
Change-Id: I1d19a0e9ff4172435f4b2b9bbe3bf72a93e2411c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3972179

Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
Reviewed-by: Leszek Swirski <les...@chromium.org>

Git Watcher via monorail

unread,
Nov 8, 2022, 10:38:05 AM11/8/22
to v8-re...@googlegroups.com

Comment #44 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c44


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/24bddb56822af9d3a77e437ee626aef52741fe0d

commit 24bddb56822af9d3a77e437ee626aef52741fe0d
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Mon Nov 07 21:30:34 2022

Merge with cached Script after synchronous deserialization

Currently, if a script is deserialized on a background thread and a
matching Script object is found in the Isolate compilation cache, the
new content is merged into the existing Script. This CL implements the
same merging for the much simpler case of deserializing on the main
thread. I expect speed changes to be minimal, because merging is only
needed in a small minority of compilations. When needed, it usually
takes about 10% as long as the corresponding deserialization.

Bug: v8:12808
Change-Id: Ie7a92bcb3111edf4cdab0eddeb7567979b35f437
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4010100

Reviewed-by: Leszek Swirski <les...@chromium.org>
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>

Git Watcher via monorail

unread,
Nov 8, 2022, 1:31:10 PM11/8/22
to v8-re...@googlegroups.com

Comment #45 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c45


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/c37c2561e208ee6a019120d8852e7b97a8d2fef2

commit c37c2561e208ee6a019120d8852e7b97a8d2fef2
Author: Michael Achenbach <mache...@chromium.org>
Date: Tue Nov 08 18:27:51 2022

Revert "Merge with cached Script after synchronous deserialization"

This reverts commit 24bddb56822af9d3a77e437ee626aef52741fe0d.

Reason for revert: Speculative revert for flaky crashes on gpu bots:
https://ci.chromium.org/ui/p/v8/builders/ci/Win%20V8%20FYI%20Release%20(NVIDIA)/17029/overview
https://ci.chromium.org/ui/p/v8/builders/ci/Mac%20V8%20FYI%20Release%20(Intel)/19158/overview

More details and dcheck failure:
https://chromium-swarm.appspot.com/task?id=5e70eb442722ba10

Original change's description:

> Merge with cached Script after synchronous deserialization
>
> Currently, if a script is deserialized on a background thread and a
> matching Script object is found in the Isolate compilation cache, the
> new content is merged into the existing Script. This CL implements the
> same merging for the much simpler case of deserializing on the main
> thread. I expect speed changes to be minimal, because merging is only
> needed in a small minority of compilations. When needed, it usually
> takes about 10% as long as the corresponding deserialization.
>
> Bug: v8:12808
> Change-Id: Ie7a92bcb3111edf4cdab0eddeb7567979b35f437
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4010100
> Reviewed-by: Leszek Swirski <les...@chromium.org>
> Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
> Cr-Commit-Position: refs/heads/main@{#84123}

Bug: v8:12808
Change-Id: I20c9a17db23a4fefae9782962156bd0807f084b8

No-Presubmit: true
No-Tree-Checks: true
No-Try: true

Git Watcher via monorail

unread,
Nov 14, 2022, 5:27:08 PM11/14/22
to v8-re...@googlegroups.com

Comment #46 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c46


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/de96cb1552438e067f8727ac5f263fa5d649da4b

commit de96cb1552438e067f8727ac5f263fa5d649da4b
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Wed Nov 09 19:25:36 2022

Reland "Merge with cached Script after synchronous deserialization"

Changes since original:
- Updated to use the returned value from CompleteMergeInForeground as
the compilation result, which is important for correctness.
- Added a test to verify the above.
- Moved the merge code into code-serializer.cc so that it can run before
FinalizeDeserialization, which makes it more consistent with
background deserialization.


Original change's description:
> Merge with cached Script after synchronous deserialization
>
> Currently, if a script is deserialized on a background thread and a
> matching Script object is found in the Isolate compilation cache, the
> new content is merged into the existing Script. This CL implements the
> same merging for the much simpler case of deserializing on the main
> thread. I expect speed changes to be minimal, because merging is only
> needed in a small minority of compilations. When needed, it usually
> takes about 10% as long as the corresponding deserialization.
>
> Bug: v8:12808
> Change-Id: Ie7a92bcb3111edf4cdab0eddeb7567979b35f437
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4010100
> Reviewed-by: Leszek Swirski <les...@chromium.org>
> Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
> Cr-Commit-Position: refs/heads/main@{#84123}

Bug: v8:12808
Change-Id: I0628a381644e79888cb3ebdd97bda270814d0e9b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4014644

Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
Reviewed-by: Leszek Swirski <les...@chromium.org>

Git Watcher via monorail

unread,
Nov 21, 2022, 11:05:07 AM11/21/22
to v8-re...@googlegroups.com

Comment #47 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c47


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/1c90992ffc2c6d12b9a130c1b7c93ff869bb5b7f

commit 1c90992ffc2c6d12b9a130c1b7c93ff869bb5b7f
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Wed Nov 16 18:18:33 2022

Merge with cached Script after streaming compilation

Currently, if a script is compiled on the main thread or deserialized on
any thread, and a matching Script object is found in the Isolate

compilation cache, the new content is merged into the existing Script.
This CL implements the same merging for scripts which were compiled by a
background task. I expect speed changes to be minimal, because merging

is only needed in a small minority of compilations. When needed, it
usually takes about 10% as long as the deserialization of the script,
which in turn is faster than compilation from source text.

This CL also removes some code which I added in preparation for merging
on a background thread in this case. Upon further discussion, we've
determined that the extra round trip to a background thread when the
main thread is likely just waiting for completion would do more harm
than good, and performing the compilation cache lookup from the
background thread would be quite cumbersome.

Bug: v8:12808
Change-Id: Ia7a14a739779ab658b505572d19df4ec489a078e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4023904

Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
Reviewed-by: Leszek Swirski <les...@chromium.org>

Git Watcher via monorail

unread,
Nov 27, 2023, 12:30:10 PM11/27/23
to v8-re...@googlegroups.com

Comment #48 on issue 12808 by Git Watcher: Allow Isolate script cache to reuse Scripts even after root SharedFunctionInfos are cleared
https://bugs.chromium.org/p/v8/issues/detail?id=12808#c48


The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/f069b82296eea932b4c0df670ff0d067e5e83a1f

commit f069b82296eea932b4c0df670ff0d067e5e83a1f
Author: Seth Brenith <seth.b...@microsoft.com>
Date: Thu Nov 02 17:46:24 2023

Tell the embedder about the Isolate compilation cache

This change adds a way that the embedder can observe whether V8 used
data from its in-memory compilation cache when compiling a script. My
short-term goal is to surface this information in the "Compile Script"
events in the devtools timeline. Longer term, this pattern may allow us
to fix the TODO comment on CacheBehaviour, which says that we should
push such data back up the API so that Blink can be responsible for
logging it. I propose using the existing Source and StreamedSource
classes to contain the data which V8 is sending to Blink, because the
API functions for compilation already have many parameters and I'm
reluctant to add another.

Bug: v8:12808
Change-Id: Ibcd81007024d6502e32e5fcd64022fba439ec7b4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4998430
Reviewed-by: Leszek Swirski <les...@chromium.org>
Reviewed-by: Toon Verwaest <verw...@chromium.org>
Commit-Queue: Seth Brenith <seth.b...@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#91198}

[modify] https://crrev.com/f069b82296eea932b4c0df670ff0d067e5e83a1f/test/cctest/test-serialize.cc
[modify] https://crrev.com/f069b82296eea932b4c0df670ff0d067e5e83a1f/test/cctest/test-api.cc
[modify] https://crrev.com/f069b82296eea932b4c0df670ff0d067e5e83a1f/include/v8-script.h
[modify] https://crrev.com/f069b82296eea932b4c0df670ff0d067e5e83a1f/src/codegen/compiler.h
[modify] https://crrev.com/f069b82296eea932b4c0df670ff0d067e5e83a1f/test/unittests/compiler/compiler-unittest.cc
[modify] https://crrev.com/f069b82296eea932b4c0df670ff0d067e5e83a1f/src/codegen/compiler.cc
[modify] https://crrev.com/f069b82296eea932b4c0df670ff0d067e5e83a1f/src/debug/debug-evaluate.cc
[modify] https://crrev.com/f069b82296eea932b4c0df670ff0d067e5e83a1f/test/cctest/compiler/test-linkage.cc
[modify] https://crrev.com/f069b82296eea932b4c0df670ff0d067e5e83a1f/src/api/api.cc
[modify] https://crrev.com/f069b82296eea932b4c0df670ff0d067e5e83a1f/src/debug/debug-interface.cc
[modify] https://crrev.com/f069b82296eea932b4c0df670ff0d067e5e83a1f/src/init/bootstrapper.cc
Reply all
Reply to author
Forward
0 new messages