There are two changes in this patch.
1. We previously added `VerifyRegExpSyntax` in regexp-parser.h to support checking regexp syntax for early errors in SpiderMonkey. Now that V8 is also emitting early errors for regexps (bug v8:896), SpiderMonkey can use the same code as V8.
2. Bug v8:11069 used a std::unordered_map as a cache for range arrays. This is currently the only place in irregexp that can call non-placement new, which SpiderMonkey has a static analysis to detect. Converting this to a ZoneUnorderedMap solves the problem for us, and seems consistent with the rest of irregexp.
Bug: v8:13108
Change-Id: Icedafd7d30fd040760cb0676a7bef8d55853bb93
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3785444
Commit-Queue: Jakob Linke <jgruber@chromium.org>
Reviewed-by: Jakob Linke <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81988}
After some solid bike shedding, we decided to rename one part of the
API.
R=jarin@chromium.org
Bug: chromium:1334585
Change-Id: Ie967f9f4947b2c328433e4c4a9d748ad15ae7175
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3788095
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81987}
Remove the range check of formatRange, formatRangeToParts on
NumberFormat and selectRange on PluralRules
Bug: v8:10776
Change-Id: Ifede7d61db6414d5b338b22bd188406e5f7d98b7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3779041
Reviewed-by: Shu-yu Guo <syg@chromium.org>
Commit-Queue: Frank Tang <ftang@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81984}
Call Intl function which call ICU TimeZone for the calculation
of timezone other than UTC
Bug: v8:11544
Change-Id: Idc355aaeccc0bed026a7117bb366ee914fa29733
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3783074
Commit-Queue: Frank Tang <ftang@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81983}
Using the external pointer table when compressing pointers lets us ease
the alignment requirement for the state field from 8-byte-aligned to
4-byte-aligned, as 8-byte alignment is not supported during compaction.
Bug: v8:12547
Change-Id: Ibbcb0d71f09f9bac66acc81459ab71e354ea405f
Cq-Include-Trybots: luci.v8.try:v8_linux64_pointer_compression_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3783077
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81979}
This CL moves the external pointer table out of V8_ENABLE_SANDBOX and
into V8_COMPRESS_POINTERS. The external pointer table is also useful
even when not sandboxing external pointers to ease alignment
requirements under pointer compression.
It is onerous for the allocator to support non-tagged-size alignment.
Under pointer compression, tagged is 4 bytes while system pointers are
8 bytes. Because external pointer table indices are 4-bytes, fields that
require natural alignment (e.g. the state field in JSAtomicsMutex) when
the system pointer size is 8-bytes can use an indirection via the
pointer table to ease the alignment restriction back to 4-bytes under
pointer compression.
Bug: v8:10391
Change-Id: Iac1200e40c987128cd6a227cd279ba4dac0e5c56
Cq-Include-Trybots: luci.v8.try:v8_linux64_pointer_compression_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3783076
Reviewed-by: Samuel Groß <saelo@chromium.org>
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81977}
Similar to the FullGC marking hierarchy (MarkingVisitorBase, with
derived classes MainMarkingVisitor and ConcurrentMarkingVisitor),
this CL introduces YoungGenerationMarkingVisitorBase +
YoungGenerationConcurrentMarkingVisitor, and refactors
YoungGenerationMarkingVisitor to inherit from
YoungGenerationMarkingVisitorBase.
YoungGenerationConcurrentMarkingVisitor dispatches to functions
refactored to ConcurrentMarkingVisitorUtility by the previous CL.
Bug: v8:13012
Change-Id: I0e827eb95732ed9ddf027fe68e25a0839cdda773
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3780524
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Commit-Queue: Leon Bettscheider <bettscheider@google.com>
Cr-Commit-Position: refs/heads/main@{#81976}
This is a reland of commit 60e9b50374
Original change's description:
> cppgc: Consistently treat sentinel pointer as live
>
> Sentinel pointers would be treated as live by the GC (through
> `HandleWeak()` but would be treated as dead when checked explicitly
> through the `LivenessBroker` in e.g. custom callbacks.
>
> Treat sentinel pointers as live consistently across all callsites
> and weak types.
>
> Change-Id: I9a4c096ddac1a111df808f3683325b55e7597eea
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3782800
> Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
> Reviewed-by: Omer Katz <omerkatz@chromium.org>
> Reviewed-by: Anton Bikineev <bikineev@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#81916}
Change-Id: Ie2476345b9ea8406015a3b07bd6880c1159ede08
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3779913
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81975}
Handle the case where getting a value in the iterable object that
encodes the values throws an exception.
R=ahaas@chromium.org
Bug: chromium:1347073
Change-Id: Ie660ab04148d5fd3508397ae6e08130496f61b74
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3788097
Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81974}
When the LeakSanitizer (LSan) runs, it scans all reachable memory
looking for pointers to other (live) objects, then reports all objects
that are still allocated but not reachable as leaked.
When the external pointer table is used, the pointers stored in it do
unfortunately not look like pointers to LSan as they will have some of
the top bits set. As such, LSan ignores them and may afterwards
incorrectly report some referenced objects as leaked.
To fix this, we now use a "shadow table" when LSan is active which
contains the raw pointer for every (tagged) pointer stored in the real
table. LSan can then scan this table and find all references.
Bug: v8:10391
Change-Id: If0c8b042fdd775ac3c8025d5688e62df37532ec3
Cq-Include-Trybots: luci.v8.try:v8_linux64_heap_sandbox_dbg_ng,v8_linux_arm64_sim_heap_sandbox_dbg_ng
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3779915
Commit-Queue: Samuel Groß <saelo@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81972}
This is a follow-up to https://chromium-review.googlesource.com/c/v8/v8/+/3776693
The previous CL accidentally didn't manage to enable
the PGO build because `v8_optimized_debug` is true in
release builds.
Bug: v8:10470
Change-Id: I66ec74e71eb697aa48b981d78ad526f4b8c5a917
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3784989
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Auto-Submit: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81971}
{NativeModule::GetJumpTableOffset} is totally independent of the
{NativeModule}, hence should be an independent function. It's now
defined in wasm-module.h, and works only on the WasmModule and the
function index.
{NativeModule::GetCallTargetForFunction} only has a single caller, which
is the wasm instance object, which already has direct access to the jump
table start. Hence we can just add the jump table offset there, and do
not need another helper method. This also makes it more clear that we
are returning a jump table slot.
R=jkummerow@chromium.org
Change-Id: If2e4eb4b3622df08ba905bd10783199bbb59d50a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3781348
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81970}
This extends the ExpectedOutProc runner to print a diff between the
expected and actual output. The behavior of other runners is unchanged.
Change-Id: If2b89d39cf98b8d257b1a209b5471a79ec3868ff
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3771641
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Liviu Rau <liviurau@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Auto-Submit: Matthias Liedtke <mliedtke@google.com>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81966}
The implicit copy constructor triggers a deprecation warning if the
struct contains a deprecated field. We can fix this by explicitly
declaring the copy and move constructors and assignment operators
with the deprecation warning disabled.
This CL also adds a test to check that we can indeed call the
constructors and assignment operators, which did not work before.
R=leszeks@chromium.org
Bug: v8:13092
Change-Id: Ia63ff9375de13fc6e5b5a8d59d827a742c99fb39
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3785145
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81962}
This breaks up the existing {max_mem_pages()} method and the
{kSpecMaxMemoryPages} and {kV8MaxWasmMemoryPages} constants into two
versions for memory32 and memory64, respectively.
For now, the limits are still the same.
Some checks and clamping is moved to earlier places where we still have
the information whether a memory is 32 or 64 bit.
We also store that information in the WasmMemoryObject and use this for
knowing the maximum for growing.
This CL is not supposed to change any observable behaviour.
R=jkummerow@chromium.org
Bug: v8:10949
Change-Id: Ieaca0596d1a24ef2746842954a75188494103eb2
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3782677
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81960}
All its uses are followed by more specific type checks anyway.
Change-Id: Ib3c0ca49d3c9fda672273edbe16e1ec363254e9c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3784592
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81959}
Allow calls to not specify a safepoint. When we look up the safepoint
for such a location, we return a default safepoint which just has the
stack slots and no deopt info or pushed registers. This is different to
the TF safepoint elision, which tries to find the _next_ safepoint for a
PC. This allows us to let most calls not specify a safepoint at all, at
the cost of not being able to deduplicate safepoints anymore.
Bug: v8:7700
Change-Id: Ia119f56f40b5af426e0daa521801e6386b28ddb0
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3770106
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81958}
This CL refactors WasmInternalFunction to no longer inherit from Foreign
but instead contain a (sandboxed) ExternalPointer field for the call target.
Bug: v8:10391
Change-Id: Iaaf25e635a275d7570e09699be3c8dec6108d4b3
Cq-Include-Trybots: luci.v8.try:v8_linux64_heap_sandbox_dbg_ng,v8_linux_arm64_sim_heap_sandbox_dbg_ng
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3782675
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Samuel Groß <saelo@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81957}
Avoid allocating dead (zero live range) phis, or their inputs. We should
figure out a way to remove them from the graph entirely, e.g. in a
separate DCE phase, but for now the easiest thing to do is to skip over
them.
Note that we can't eliminate them as part of the current node processing
pass, since that's the thing that records live ranges in the first
place.
Bug: v8:7700
Change-Id: I3e7f1f2214100def9ccc2b3f008852d5d69f548f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3784985
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81954}
This is a reland of commit 6925bc095f
Original change's description:
> [heap] Refactor methods and nested class of ConcurrentMarkingVisitor
>
> This CL moves a few methods and nested class SlotSnapshottingVisitor
> from ConcurrentMarkingVisitor to ConcurrentMarkingVisitorUtility.
>
> Methods in ConcurrentMarkingVisitorUtility are now static and instead have a Visitor parameter.
>
> This is preparatory work for adding a
> YoungGenerationConcurrentMarkingVisitor class, which will be able to
> reuse members of ConcurrentMarkingVisitorUtility.
>
> Bug: v8:13012
> Change-Id: I503c20e655578031018a2e37dd92c1d61bbe1686
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3779677
> Commit-Queue: Leon Bettscheider <bettscheider@google.com>
> Reviewed-by: Omer Katz <omerkatz@chromium.org>
> Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#81931}
Bug: v8:13012
Change-Id: I05063263d831ef4f3e297289e4210850029f7607
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3780500
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Commit-Queue: Leon Bettscheider <bettscheider@google.com>
Cr-Commit-Position: refs/heads/main@{#81953}
This CL refactors the mechanism for testing the implementation of inner
pointer resolution using the marking bitmap. It allows for more than one
page, where objects can be allocated. It also keeps a list of allocated
objects that are automatically tested.
Bug: v8:12851
Change-Id: I470dc1154aca1ebc3d8526872717747829f83396
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3784605
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Commit-Queue: Nikolaos Papaspyrou <nikolaos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81952}
This CL allows the PagedNewSpace to be used with flag
v8_enable_inner_pointer_resolution_osb.
Bug: v8:12612
Bug: v8:12851
Change-Id: I63eea4e75398ffec38e562cce245394c0d2a637c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3782670
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Nikolaos Papaspyrou <nikolaos@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81951}
The intial version of the API was replaced with a more ergonomic one
recently.
We can also safely remove the test as the new API guarantees that
tasks are always finished and cancelled.
Bug: chromium:1334585
Change-Id: I9ff8b92fcd73ef821c86de52c40a1d04b15ea918
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3780539
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81950}
This reverts commit 4e935c7ffb.
Reason for revert: Breaking on mac arm64: https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Mac%20-%20arm64%20-%20release/10926/overview
Original change's description:
> [pku][wasm] Refactor PKU usage in Wasm
>
> RwxMemoryWriteScope becomes the bottleneck for both MAP_JIT and PKU
> machinery.
> Wasm and V8 code space will use the same memory protection key.
>
> This is a next step towards adding PKU support for V8 code space.
>
> Bug: v8:13023
> Change-Id: I647f8c09bc41e5ef8a1d74b58a48a43e08454e0d
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3702213
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Commit-Queue: Wenqin Yang <wenqin.yang@intel.com>
> Reviewed-by: Igor Sheludko <ishell@chromium.org>
> Reviewed-by: Clemens Backes <clemensb@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#81947}
Bug: v8:13023
Change-Id: I11c52ac101804ab75b1bb1d4814f083cb1083d5b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3780498
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Owners-Override: Leszek Swirski <leszeks@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#81949}
RwxMemoryWriteScope becomes the bottleneck for both MAP_JIT and PKU
machinery.
Wasm and V8 code space will use the same memory protection key.
This is a next step towards adding PKU support for V8 code space.
Bug: v8:13023
Change-Id: I647f8c09bc41e5ef8a1d74b58a48a43e08454e0d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3702213
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Wenqin Yang <wenqin.yang@intel.com>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81947}
This reverts commit 6925bc095f.
Reason for revert: Speculative revert for roll failures: https://ci.chromium.org/ui/p/chromium/builders/try/linux_optional_gpu_tests_rel/85744/overview
Original change's description:
> [heap] Refactor methods and nested class of ConcurrentMarkingVisitor
>
> This CL moves a few methods and nested class SlotSnapshottingVisitor
> from ConcurrentMarkingVisitor to ConcurrentMarkingVisitorUtility.
>
> Methods in ConcurrentMarkingVisitorUtility are now static and instead have a Visitor parameter.
>
> This is preparatory work for adding a
> YoungGenerationConcurrentMarkingVisitor class, which will be able to
> reuse members of ConcurrentMarkingVisitorUtility.
>
> Bug: v8:13012
> Change-Id: I503c20e655578031018a2e37dd92c1d61bbe1686
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3779677
> Commit-Queue: Leon Bettscheider <bettscheider@google.com>
> Reviewed-by: Omer Katz <omerkatz@chromium.org>
> Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#81931}
Bug: v8:13012
Change-Id: If2240b2e0769b04d752caefceb95609c6b950bb2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3787373
Owners-Override: Deepti Gandluri <gdeepti@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Deepti Gandluri <gdeepti@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81945}
This reverts commit e895b7af73.
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:
> 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 <leszeks@chromium.org>
> Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
> Cr-Commit-Position: refs/heads/main@{#81941}
Bug: v8:12808
Change-Id: I82a080e6287828445293cb6b4b94a5e8f15eb8f3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3787213
Auto-Submit: Deepti Gandluri <gdeepti@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Deepti Gandluri <gdeepti@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#81943}
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 <leszeks@chromium.org>
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#81941}
