Skip to content

V
V8
Switch branch/tag
  1. 07 Jan, 2021 16 commits
  3. 06 Jan, 2021 12 commits
  5. 05 Jan, 2021 12 commits
    • Sara Tang's avatar
      Step 1 (of 3-ish): Basic ETW Instrumentation in V8 · 8b33c872
      Sara Tang authored 
      Design doc:
https://docs.google.com/document/d/1xkXj94iExFgLWc_OszTNyNGi523ARaKMWPZTeomhI4U
A lot has changed since the last patchset! I recommend revisiting this
design doc and reading the parts in green. I explain the roadmap for
what changes to expect from ETW instrumentation as well as the
instrumentation of this particular CL.

I'll do my best to answer any further questions anyone has about my
particular instrumentation or ETW in general :)

---

This is the first of a series of changelists to round out ETW
instrumentation for V8.

This changelist represents the most minimal change needed to instrument
ETW in V8. In particular, it:
  - defines and registers the ETW provider,
  - interacts minimally with the rest of V8, by hooking into the
    existing TracingController::AddTraceEvent function,
  - is designed with a platform-agnostic layer, so that event tracers
    for other platforms can be instrumented in teh future.

Some notes on instrumentation (aka I copied stuff from the design doc):

We make heavy use of the TraceLogging API to log events. It differs from
previous methods of emitting ETW events in that it doesn<E2><80><99>t
require the overhead of a separate manifest file to keep track of
metadata; rather, events using this API are self-descriptive.

Here are the five major steps to instrument the TraceLogging API:
  - Forward declare the provider (from provider-win.h)
  - Define the provider in a .cc file (from provider-win.cc)
  - Register the provider (called from v8.cc).
  - Write events (called from libplatform/tracing-controller.cc)
  - Unregister the provider (called from v8.cc)

At the base, we have an abstract provider class that encapsulates the
functionality of an event provider. These are things like registering
and unregistering the provider, and the actual event-logging.

The provider class is split into provider-win and provider-mac
(currently not instantiated) classes, with OS-dependent implementations
of the above functions.

In particular, the TraceLogging API is used only in provider-win. It is
here that we forward declare and define the provider, as well as write
ETW events.

Finally, there is a v8-provider class that serves as a top-level API and
is exposed to the rest of V8. It acts as a wrapper for the
platform-specific providers.

The .wprp file is needed so that Windows Performance Recorder knows how
to capture our events.

Some considerations:
  - Is TracingController::AddTraceEvent the best place from which to
    write my events?
  - Is src/libplatform/tracing the best place to put my instrumentation?
  - Right now, I fail the preupload because of this, which tells me my
    files are probably not in the best location:

You added one or more #includes that violate checkdeps rules.
src\init\v8.cc Illegal include: "src/libplatform/tracing/v8-provider.h"
Because of "-src/libplatform" from src's include_rules.

Change-Id: Id53e4a034c9e526524a17000da0a647a95d93edf
Bug: v8:11043
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2233407Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
Reviewed-by: 's avatarPeter Marshall <petermarshall@chromium.org>
Commit-Queue: Sara Tang <sartang@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#71918}
      8b33c872
    • Andreas Haas's avatar
      [wasm] Remove negative implication from --single-threaded to --liftoff · 3345472f
      Andreas Haas authored 
      The negative implication from --single-threaded to --liftoff was
introduced because with --single-threaded, no tier-up to TurboFan is
possible, and TurboFan provides faster code and smaller code

This CL removes this implication. The reason is that this implication
does not only define the default compiler for --single-threaded to
TurboFan, and also completely disables the option to use Liftoff
instead. By removing the implication, and embedder who uses
--single-threaded can decide by themselves if they want to use Liftoff
by setting --liftoff, or if they don't want to use Liftoff by setting
--no-liftoff.

R=clemensb@chromium.org

Bug: v8:11279
Change-Id: Ie6e64a42823b87484135364ecb4589cfd188db5e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2599548Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71917}
      3345472f
    • Clemens Backes's avatar
      Revert "[wasm-simd][liftoff][x64] Move v128.select into macro-assembler" · b48824ff
      Clemens Backes authored 
      This reverts commit 2d5f981a.

Reason for revert: Fails on noavx: https://ci.chromium.org/p/v8/builders/ci/V8%20Linux64%20-%20debug/35318

Original change's description:
> [wasm-simd][liftoff][x64] Move v128.select into macro-assembler
>
> This allows us to reuse this optimized code sequence in Liftoff.
>
> We can't do the same thing in IA32 yet, there is no kScratchDoubleReg
> defined in the macro-assembler-ia32.cc, it is defined in code-generator-ia32
> as xmm0 but I'm not sure if it is safe to just use that in the macro assembler.
>
> Change-Id: I6c761857c49d2518fbc82cd0796c62fc86665cb5
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2596581
> Commit-Queue: Zhi An Ng <zhin@chromium.org>
> Reviewed-by: Clemens Backes <clemensb@chromium.org>
> Reviewed-by: Bill Budge <bbudge@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#71915}

TBR=bbudge@chromium.org,clemensb@chromium.org,zhin@chromium.org

Change-Id: I2aacee02c89a16516a9cd6686d8cc6180362f78e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2610730Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71916}
      b48824ff
    • Zhi An Ng's avatar
      [wasm-simd][liftoff][x64] Move v128.select into macro-assembler · 2d5f981a
      Zhi An Ng authored 
      This allows us to reuse this optimized code sequence in Liftoff.

We can't do the same thing in IA32 yet, there is no kScratchDoubleReg
defined in the macro-assembler-ia32.cc, it is defined in code-generator-ia32
as xmm0 but I'm not sure if it is safe to just use that in the macro assembler.

Change-Id: I6c761857c49d2518fbc82cd0796c62fc86665cb5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2596581
Commit-Queue: Zhi An Ng <zhin@chromium.org>
Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
Reviewed-by: 's avatarBill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71915}
      2d5f981a
    • Clemens Backes's avatar
      [logging] Fix expected death message in official builds · bdcdbd4b
      Clemens Backes authored 
      The tests were failing in official release builds, because those drop
the fatal error message and always print "ignored" instead.

R=ecmziegler@chromium.org

Bug: v8:11251
Change-Id: I40512ca308337cf070ecb6a206dc4a5323d67415
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2595445Reviewed-by: 's avatarEmanuel Ziegler <ecmziegler@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71914}
      bdcdbd4b
    • Clemens Backes's avatar
      [factory] Allow undefined source for scripts · d0ebe06b
      Clemens Backes authored 
      According to the Torque definition, the type of the 'source' field is
'String|Undefined'. The Factory only allowed to pass a string though,
which forced us to set an empty string for wasm scripts.

This CL changes the factory to also allow undefined values, which fits
slightly better for wasm.
The inspector needed a minor change to handle undefined source like an
empty string.

R=dinfuehr@chromium.org, yangguo@chromium.org

Bug: chromium:1125986
Change-Id: Iac0a5ee3767ce121aba8a6a2afe37195e77122fa
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2584243Reviewed-by: 's avatarDominik Inführ <dinfuehr@chromium.org>
Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71913}
      d0ebe06b
    • Zhi An Ng's avatar
      Add comment discussing Macro v.s. Turbo Assemblers · 77a77aea
      Zhi An Ng authored 
      R=ahaas@chromium.org

Change-Id: Iffc27328464616b422cb6ccecbaf77d901f8d889
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2599848
Commit-Queue: Zhi An Ng <zhin@chromium.org>
Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71912}
      77a77aea
    • Benedikt Meurer's avatar
      [debug][wasm] Move DebugEvaluate implementation to wasm-debug.cc. · 5fc4649d
      Benedikt Meurer authored 
      Previously the proxies that make up the DebugEvaluate implementation for
Wasm frames lived in wasm-js.cc, but that was quite confusing since

(a) the rest of the debug support for Wasm lives in wasm-debug.cc (and
    we intend to eventually unify the DebugEvaluate and Scope objects),
    and
(b) the wasm-js.cc file is explicitly about the WebAssembly JS API
    that's part of the WebAssembly specification, and the DebugEvaluate
    proxies aren't part of that.

Bug: chromium:1162229, chromium:1071432, chromium:1127914
Change-Id: I63016dcace6d8e2af4a03c8eed4f02d464c1dee1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2609418
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Zhi An Ng <zhin@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: 's avatarZhi An Ng <zhin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71911}
      5fc4649d
    • Benedikt Meurer's avatar
      [inspector] Add Wasm test coverage for evaluateOnCallFrame(). · d09b35f3
      Benedikt Meurer authored 
      Drive-by-fix: Handle duplicate globals names correctly in the
scope exposed module object.

Bug: chromium:1127914, chromium:1071432
Change-Id: I697256642c5ddbc13f86ff25ab012c53537b9c88
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2609416
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71910}
      d09b35f3
    • Benedikt Meurer's avatar
      [inspector] Remove redundant tests. · adea008b
      Benedikt Meurer authored 
      The wasm-scope-info-liftoff.js and wasm-set-breakpoint-liftoff.js tests
were originally testing the Liftoff path (when we still had the Wasm
interpreter), and have received some updates along the way. Nowadays the
interpreter is going and the non-liftoff versions of these tests don't
provide any additional test coverage, but are merely a slightly less
updated version of the liftoff test.

Bug: chromium:1162229
Change-Id: Ifc9933d47f33674a83b99425ef9d0e4bc5550323
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2609415
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71909}
      adea008b
    • Benedikt Meurer's avatar
      [inspector] Make wasm tests fail rather than time out. · 6f448efb
      Benedikt Meurer authored 
      Consistently use InspectorTest.runAsyncTestSuite() in wasm inspector
tests to make tests easier to debug (they'll fail instead of timing
out in case of errors).

Bug: chromium:1162229, chromium:1071432
Change-Id: I7aada196f9e34071aa1bb059bb45f85f75226060
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2609414
Commit-Queue: Yang Guo <yangguo@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71908}
      6f448efb
    • Zhi An Ng's avatar
      [wasm-simd][ia32] Convert ext mul macros into macro-assembler functions · afd3f63e
      Zhi An Ng authored 
      This will make these functions usable from Liftoff when we later
implement extended multiply instructions in Liftoff.

This is similar to the x64 versions, except that it takes a scratch
register as a parameter.

Bug: v8:11262
Change-Id: Ief3d8cdde59da9e05a468286315bcae6d13863d9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2603768Reviewed-by: 's avatarBill Budge <bbudge@chromium.org>
Commit-Queue: Zhi An Ng <zhin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71907}
      afd3f63e