Commit Graph

16 Commits

Author SHA1 Message Date
Clemens Backes
f368298445 [wasm] Initialize compile job early
Since the compile job can always be reused after creation (even if it
runs out of work), we do not need the logic to (re-)initialize it. In
fact, it will always only be initialized once already.
This allows us to initialize it once during construction of the
compilation state (or right after the initialization), and then access
it without locks later.

In addition, this CL
1) renames "current_compile_job_" to "compile_job_", since there will
   always only be one now;
2) removes the {ScheduleCompileJobForNewUnits} method, and just does a
   {compile_job_->NotifyConcurrencyIncrease()} instead;
3) removes the {has_priority_} field and just directly does a
   {compile_job_->UpdatePriority} call.

The streaming test platform needed to be fixed to avoid calling {Join}
on the job handle, which would invalidate the handle afterwards.
Instead, we just run all tasks as long as there are any.

R=thibaudm@chromium.org
CC=etiennep@chromium.org

Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel_ng
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_isolates_rel_ng
Change-Id: I7094231e86d5f54cfca5e971b96fd81e994c874a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2584946
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71757}
2020-12-15 09:43:14 +00:00
Etienne Pierre-doray
72249208e7 [Jobs]: Deprecate IsCompleted and IsRunning.
Follow up on https://chromium-review.googlesource.com/c/v8/v8/+/2510969
Now that gin implements the new version:
https://chromium-review.googlesource.com/c/chromium/src/+/2566052
These can be deprecated.

Change-Id: Ie1e5448655e40eb3c11089f59510f269a9873e66
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2566430
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71700}
2020-12-10 16:48:48 +00:00
Etienne Pierre-doray
fc1d6f35ef Reland "Reland "[wasm]: Use CancelAndDetach and barrier on BackgroundCompileJob.""
This is a reland of 064ee3c835

Issue 1: WasmEngine UAF when CompilationState is destroyed
asynchronously
Fix: Include https://chromium-review.googlesource.com/c/v8/v8/+/2565508
in this CL. Use OperationBarrier to keep WasmEngine alive.

Issue 2: In gin, JobTask lifetime is not extended beyond
JobHandle, thus making CancelAndDetach unusable.
This is fixed in chromium here:
https://chromium-review.googlesource.com/c/chromium/src/+/2566724

Original change's description:
> Reland "[wasm]: Use CancelAndDetach and barrier on BackgroundCompileJob."
>
> Reason for revert: Data race:
> https://ci.chromium.org/p/v8/builders/ci/V8%20Linux64%20TSAN/34121
>
> It was assume that MockPlatform runs everything on 1 thread. However,
> MockPlatform::PostJob previously would schedule the job through
> TestPlatform, which eventually posts concurrent tasks, thus causing
> data race.
> Fix: Manually calling NewDefaultJobHandle and passing the MockPlatform
> ensures the jobs also run sequentially.
>
> Additional change:
> - CancelAndDetach is now called in ~CompilationStateImpl() to make sure
> it's called in sequence with ScheduleCompileJobForNewUnits
>
> Original CL description:
> To avoid keeping around a list of job handles, CancelAndDetach() is
> used in CancelCompilation. Dependency on WasmEngine is handled by a
> barrier that waits on all jobs to finish.
>
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2498659
> Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Reviewed-by: Clemens Backes <clemensb@chromium.org>
> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
> Cr-Original-Commit-Position: refs/heads/master@{#71074}
> Change-Id: Ie9556f7f96f6fb9a61ada0e5cbd58d4fb4a0f571
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2559137
> Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
> Reviewed-by: Andreas Haas <ahaas@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#71459}

TBR=ulan@chromium.org

Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel_ng
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_isolates_rel_ng
Change-Id: I6175092c97fea0d5f63a97af232e2d54cccea535
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2569360
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71662}
2020-12-08 15:42:15 +00:00
Etienne Pierre-Doray
393782efee Revert "Reland "[wasm]: Use CancelAndDetach and barrier on BackgroundCompileJob.""
This reverts commit 064ee3c835.

Reason for revert: Causing blink_web_tests to fail on builder "WebKit Linux MSAN"
https://bugs.chromium.org/p/chromium/issues/detail?id=1153968

Original change's description:
> Reland "[wasm]: Use CancelAndDetach and barrier on BackgroundCompileJob."
>
> Reason for revert: Data race:
> https://ci.chromium.org/p/v8/builders/ci/V8%20Linux64%20TSAN/34121
>
> It was assume that MockPlatform runs everything on 1 thread. However,
> MockPlatform::PostJob previously would schedule the job through
> TestPlatform, which eventually posts concurrent tasks, thus causing
> data race.
> Fix: Manually calling NewDefaultJobHandle and passing the MockPlatform
> ensures the jobs also run sequentially.
>
> Additional change:
> - CancelAndDetach is now called in ~CompilationStateImpl() to make sure
> it's called in sequence with ScheduleCompileJobForNewUnits
>
> Original CL description:
> To avoid keeping around a list of job handles, CancelAndDetach() is
> used in CancelCompilation. Dependency on WasmEngine is handled by a
> barrier that waits on all jobs to finish.
>
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2498659
> Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Reviewed-by: Clemens Backes <clemensb@chromium.org>
> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
> Cr-Original-Commit-Position: refs/heads/master@{#71074}
> Change-Id: Ie9556f7f96f6fb9a61ada0e5cbd58d4fb4a0f571
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2559137
> Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
> Reviewed-by: Andreas Haas <ahaas@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#71459}

TBR=ulan@chromium.org,jkummerow@chromium.org,ahaas@chromium.org,clemensb@chromium.org,etiennep@chromium.org
Bug: chromium:1153968, v8:11209, v8:11210, v8:11212

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: I2c8406bea81ee7cf6c5726c2fec50fffdce09611
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2566446
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71519}
2020-12-01 09:39:31 +00:00
Etienne Pierre-doray
064ee3c835 Reland "[wasm]: Use CancelAndDetach and barrier on BackgroundCompileJob."
Reason for revert: Data race:
https://ci.chromium.org/p/v8/builders/ci/V8%20Linux64%20TSAN/34121

It was assume that MockPlatform runs everything on 1 thread. However,
MockPlatform::PostJob previously would schedule the job through
TestPlatform, which eventually posts concurrent tasks, thus causing
data race.
Fix: Manually calling NewDefaultJobHandle and passing the MockPlatform
ensures the jobs also run sequentially.

Additional change:
- CancelAndDetach is now called in ~CompilationStateImpl() to make sure
it's called in sequence with ScheduleCompileJobForNewUnits

Original CL description:
To avoid keeping around a list of job handles, CancelAndDetach() is
used in CancelCompilation. Dependency on WasmEngine is handled by a
barrier that waits on all jobs to finish.

Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2498659
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#71074}
Change-Id: Ie9556f7f96f6fb9a61ada0e5cbd58d4fb4a0f571
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2559137
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71459}
2020-11-27 17:50:15 +00:00
Sara Tang
c9e883e803 (Step 1 of 2): Prepping Wasm events in the Recorder interface
As part of an effort to prepare the Recorder interface for general use,
we had to make some changes to the way the existing Wasm Events are
being used. In particular,
  - it is more fitting to use a ElapsedTimer than a TimedScope to
    measure the durations in src/wasm/module-[decoder|instantiate].cc
  - we want to rename the wall_clock_time_in_us field to duration_in_us
    for clarity.

Because these Wasm events are already being instantiated in chromium,
renaming the field requires a two-step change. This is the first of
those changes.

Change-Id: If1b2990f7645616a59fc21d07ac10bf00701c0e5
Bug: v8:11109
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2518619
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Emanuel Ziegler <ecmziegler@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71053}
2020-11-09 15:29:14 +00:00
Etienne Pierre-doray
4fb053d847 [Jobs API] Rename !IsCompleted -> IsActive
IsCompleted is backwards. For a more consistent api, the function is
renamed IsActive and logic is flipped.
Following up on https://chromium-review.googlesource.com/c/v8/v8/+/2461840
The intend is to make the distinction between IsActive and IsValid obvious.

Change-Id: Iaf00b9f6ffa8f1efe93ae29f09899737ef20f04d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2510969
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70901}
2020-10-30 19:57:38 +00:00
Etienne Pierre-doray
10b847c765 [Jobs API] Rename IsRunning -> IsValid
IsRunning is the v8 equivalent of operator bool, but is confusing
with IsCompleted. IsValid (to match base:: operator bool) should be more
clear.

Change-Id: I2529bea21c7cb7613bd5057c66715fb5ea450396
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2461840
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70625}
2020-10-19 17:01:48 +00:00
Ulan Degenbaev
dae268e404 [test] Fix platform lifetime in test-wasm-metrics
Currently MockPlatform has shorter lifetime than the isolate that uses
it. This leads to use-after-free races in concurrent tasks that fetch
the mock platform just before it is freed.

This CL ensures that MockPlatform is valid throughout the whole
lifetime of the isolate

Change-Id: Ib94dc7674b9f94833be3372de68209ec38577ca1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2461726
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Emanuel Ziegler <ecmziegler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70407}
2020-10-08 15:26:51 +00:00
Etienne Pierre-doray
4f1bf7d10b [Jobs]: Expose CancelAndDetach()
This is useful for wasm instead of keeping around a list of handles.

Change-Id: I4ef970ba191a66303c577bbe8e6ab1327aad2e24
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2451209
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70353}
2020-10-06 15:54:31 +00:00
Junliang Yan
230da091d4 [wasm][ukm] relax the condition for Turbofan
On ppc64 and s390x, Liftoff is not implemented yet, so if a Liftoff
compilation unit finishes after all top-tier units (hence after the "top
tier finished" callback), it will still increase the turbofan counter.

R=clemensb@chromium.org, ecmziegler@chromium.org

Bug: chromium:1092417
Change-Id: I0b99061f26851288f1abb8fcc3a30ca92a55164e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2429564
Reviewed-by: Emanuel Ziegler <ecmziegler@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Junliang Yan <junyan@redhat.com>
Cr-Commit-Position: refs/heads/master@{#70123}
2020-09-24 17:59:37 +00:00
Emanuel Ziegler
b77c63c0a6 [wasm][ukm] Add tests for Wasm events (reland)
Ensure that events are triggered when a module is decoded, compiled,
instantiated and tiered-up.

This is a reland of Ib5883a338c3756c6f3488fbdd7b6861ecc2ba218.

R=clemensb@chromium.org
TBR=adamk@chromium.org

Bug: chromium:1092417
Change-Id: I803ae3db23a5f71f26e8ec118251eccdfc551353
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel_ng
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_isolates_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2425056
Commit-Queue: Emanuel Ziegler <ecmziegler@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70113}
2020-09-24 12:05:27 +00:00
Francis McCabe
8efe0e32b3 Revert "[wasm][ukm] Add tests for Wasm events (reland)"
This reverts commit 54b141ef9a.

Reason for revert: closed tree due to: https://ci.chromium.org/p/v8/builders/ci/V8%20Linux%20-%20noi18n%20-%20debug/33838?

Original change's description:
> [wasm][ukm] Add tests for Wasm events (reland)
> 
> Ensure that events are triggered when a module is decoded, compiled,
> instantiated and tiered-up.
> 
> This is a reland of I9dc87957fc03023c5ab1c4f49e865957c8324e1a.
> 
> R=​clemensb@chromium.org
> 
> Bug: chromium:1092417
> Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel_ng
> Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_isolates_rel_ng
> Change-Id: Ib5883a338c3756c6f3488fbdd7b6861ecc2ba218
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2367866
> Reviewed-by: Adam Klein <adamk@chromium.org>
> Reviewed-by: Clemens Backes <clemensb@chromium.org>
> Commit-Queue: Emanuel Ziegler <ecmziegler@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#70074}

TBR=adamk@chromium.org,clemensb@chromium.org,ecmziegler@chromium.org

Change-Id: I859c69bacfca9c790ed274140bb9e8d23de26729
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:1092417
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel_ng
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_isolates_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2425104
Reviewed-by: Francis McCabe <fgm@chromium.org>
Commit-Queue: Francis McCabe <fgm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70075}
2020-09-22 20:52:31 +00:00
Emanuel Ziegler
54b141ef9a [wasm][ukm] Add tests for Wasm events (reland)
Ensure that events are triggered when a module is decoded, compiled,
instantiated and tiered-up.

This is a reland of I9dc87957fc03023c5ab1c4f49e865957c8324e1a.

R=clemensb@chromium.org

Bug: chromium:1092417
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel_ng
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_isolates_rel_ng
Change-Id: Ib5883a338c3756c6f3488fbdd7b6861ecc2ba218
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2367866
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Emanuel Ziegler <ecmziegler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70074}
2020-09-22 20:21:02 +00:00
Maya Lekova
59076a3baa Revert "[wasm][ukm] Add tests for Wasm events"
This reverts commit d7b20edcb3.

Reason for revert: Introduces data races - https://ci.chromium.org/p/v8/builders/ci/V8%20Linux64%20TSAN/32898

Original change's description:
> [wasm][ukm] Add tests for Wasm events
> 
> Ensure that events are triggered when a module is decoded, compiled,
> instantiated and tiered-up.
> 
> R=​clemensb@chromium.org
> 
> Bug: chromium:1092417
> 
> Change-Id: I9dc87957fc03023c5ab1c4f49e865957c8324e1a
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2351676
> Commit-Queue: Emanuel Ziegler <ecmziegler@chromium.org>
> Reviewed-by: Clemens Backes <clemensb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#69508}

TBR=clemensb@chromium.org,ecmziegler@chromium.org

Change-Id: I9bcfeda1048939a8142f5003b03feab399f9de96
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:1092417
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2366785
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69509}
2020-08-20 15:23:33 +00:00
Emanuel Ziegler
d7b20edcb3 [wasm][ukm] Add tests for Wasm events
Ensure that events are triggered when a module is decoded, compiled,
instantiated and tiered-up.

R=clemensb@chromium.org

Bug: chromium:1092417

Change-Id: I9dc87957fc03023c5ab1c4f49e865957c8324e1a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2351676
Commit-Queue: Emanuel Ziegler <ecmziegler@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69508}
2020-08-20 14:13:40 +00:00