Commit Graph

19 Commits

Author SHA1 Message Date
Dan Elphick
ec06bb6ce5 Reland "[include] Split out v8.h"
This is a reland of d1b27019d3

Fixes include:
Adding missing file to bazel build
Forward-declaring classing before friend-classing them to fix win/gcc
Add missing v8-isolate.h include for vtune builds

Original change's description:
> [include] Split out v8.h
>
> This moves every single class/function out of include/v8.h into a
> separate header in include/, which v8.h then includes so that
> externally nothing appears to have changed.
>
> Every include of v8.h from inside v8 has been changed to a more
> fine-grained include.
>
> Previously inline functions defined at the bottom of v8.h would call
> private non-inline functions in the V8 class. Since that class is now
> in v8-initialization.h and is rarely included (as that would create
> dependency cycles), this is not possible and so those methods have been
> moved out of the V8 class into the namespace v8::api_internal.
>
> None of the previous files in include/ now #include v8.h, which means
> if embedders were relying on this transitive dependency then it will
> give compile failures.
>
> v8-inspector.h does depend on v8-scripts.h for the time being to ensure
> that Chrome continue to compile but that change will be reverted once
> those transitive #includes in chrome are changed to include it directly.
>
> Full design:
> https://docs.google.com/document/d/1rTD--I8hCAr-Rho1WTumZzFKaDpEp0IJ8ejZtk4nJdA/edit?usp=sharing
>
> Bug: v8:11965
> Change-Id: I53b84b29581632710edc80eb11f819c2097a2877
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3097448
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Reviewed-by: Camillo Bruni <cbruni@chromium.org>
> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
> Reviewed-by: Leszek Swirski <leszeks@chromium.org>
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Commit-Queue: Dan Elphick <delphick@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#76424}

Cq-Include-Trybots: luci.v8.try:v8_linux_vtunejit
Bug: v8:11965
Change-Id: I99f5d3a73bf8fe25b650adfaf9567dc4e44a09e6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3113629
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
Commit-Queue: Dan Elphick <delphick@chromium.org>
Cr-Commit-Position: refs/heads/main@{#76460}
2021-08-24 13:08:55 +00:00
Dan Elphick
44fe02ced6 Revert "[include] Split out v8.h"
This reverts commit d1b27019d3.

Reason for revert: Broke vtune build, tsan build and possibly others

Original change's description:
> [include] Split out v8.h
>
> This moves every single class/function out of include/v8.h into a
> separate header in include/, which v8.h then includes so that
> externally nothing appears to have changed.
>
> Every include of v8.h from inside v8 has been changed to a more
> fine-grained include.
>
> Previously inline functions defined at the bottom of v8.h would call
> private non-inline functions in the V8 class. Since that class is now
> in v8-initialization.h and is rarely included (as that would create
> dependency cycles), this is not possible and so those methods have been
> moved out of the V8 class into the namespace v8::api_internal.
>
> None of the previous files in include/ now #include v8.h, which means
> if embedders were relying on this transitive dependency then it will
> give compile failures.
>
> v8-inspector.h does depend on v8-scripts.h for the time being to ensure
> that Chrome continue to compile but that change will be reverted once
> those transitive #includes in chrome are changed to include it directly.
>
> Full design:
> https://docs.google.com/document/d/1rTD--I8hCAr-Rho1WTumZzFKaDpEp0IJ8ejZtk4nJdA/edit?usp=sharing
>
> Bug: v8:11965
> Change-Id: I53b84b29581632710edc80eb11f819c2097a2877
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3097448
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Reviewed-by: Camillo Bruni <cbruni@chromium.org>
> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
> Reviewed-by: Leszek Swirski <leszeks@chromium.org>
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Commit-Queue: Dan Elphick <delphick@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#76424}

Bug: v8:11965
Change-Id: Id57313ae992e720c8b19abc975cd69729e1344aa
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3113627
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Owners-Override: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#76428}
2021-08-23 11:54:09 +00:00
Dan Elphick
d1b27019d3 [include] Split out v8.h
This moves every single class/function out of include/v8.h into a
separate header in include/, which v8.h then includes so that
externally nothing appears to have changed.

Every include of v8.h from inside v8 has been changed to a more
fine-grained include.

Previously inline functions defined at the bottom of v8.h would call
private non-inline functions in the V8 class. Since that class is now
in v8-initialization.h and is rarely included (as that would create
dependency cycles), this is not possible and so those methods have been
moved out of the V8 class into the namespace v8::api_internal.

None of the previous files in include/ now #include v8.h, which means
if embedders were relying on this transitive dependency then it will
give compile failures.

v8-inspector.h does depend on v8-scripts.h for the time being to ensure
that Chrome continue to compile but that change will be reverted once
those transitive #includes in chrome are changed to include it directly.

Full design:
https://docs.google.com/document/d/1rTD--I8hCAr-Rho1WTumZzFKaDpEp0IJ8ejZtk4nJdA/edit?usp=sharing

Bug: v8:11965
Change-Id: I53b84b29581632710edc80eb11f819c2097a2877
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3097448
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Dan Elphick <delphick@chromium.org>
Cr-Commit-Position: refs/heads/main@{#76424}
2021-08-23 09:35:06 +00:00
Clemens Backes
3a44f269c5 [base] Avoid pthread_rwlock_t on Mac
pthread_rwlock_t can deadlock on Mac if signals are sent to the process
in the wrong moment. Since we use processes e.g. for sampling profiling
(in both d8 and in Chrome), we hence cannot safely use pthread_rwlock_t
on Mac. Instead, fall back to a non-shared pthread_mutex_t.

Interestingly, this shows no measurable performance impact in Wasm
compilation on my MBP.

R=mlippautz@chromium.org

Bug: v8:11399
Change-Id: Ie8bfd5288bba8c4f3315ee4502b39b59d39c9bbd
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3060480
Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#76015}
2021-07-30 10:51:09 +00:00
Clemens Backes
98ecaf6091 [cctest] Test SharedMutex plus sampling
The combination of pthread_rwlock_t and signals causes spurious
deadlocks on Mac (see linked issue).
This adds a cctest which tests this combination. This test is skipped on
Mac, where it would deadlock. This test can be used to document and
further investigate the issue, and test potential fixes.

R=jkummerow@chromium.org

Bug: v8:11399
Change-Id: I5d1fcdd84db253ec2d0637575239f212aae2ecb7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2856553
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74662}
2021-05-19 12:11:28 +00:00
Camillo Bruni
24222a9fef [api] Use shorter 8::Local::As<*> casts in more places
Bug: v8:11195
Change-Id: I19211af9e440940f85351fb38920eb620c222213
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555010
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71465}
2020-11-28 11:04:26 +00:00
Andrew Comminos
44483152c5 [cpu-profiler] Only record SIGPROF-based samples for samplers that request samples
Sets an atomic field on each sampler when it requests a sample, to be
checked when the SIGPROF handler is executed. A counter is not used
since signals may be coalesced.

Prior to this change, all samplers attached to an isolate received
samples when other samplers sent SIGPROF to the VM thread. This change
alters the behaviour of different CpuProfiler instances on the same
isolate to be in line with the Windows / Fuchsia behaviour.

Bug: v8:8835
Change-Id: I0caaa845b596efc9d8b1cd7716c067d9a6359c57
Reviewed-on: https://chromium-review.googlesource.com/c/1468941
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59545}
2019-02-13 09:29:38 +00:00
Peter Marshall
1f1bd71dd0 [cpu-profiler] Remove registration and sampling depth from Sampler
Simplify the internal state of Sampler a bit. There are basically two
users of Sampler - the CpuSampler used by the CpuProfiler and the
Ticker used by log.cc. Ticker calls Start/Stop to manage the Sampler
lifetime, but CpuProfiler does not. This leads to much confusion and
overlap of functionality.

Fix that here by removing the distinction between active, registered
and isProfiling states. These are now all the same thing and are
represented by IsActive(). The state is set to active when Start is
called, and set inactive when Stop is called. Both users of Sampler
now call Start and Stop at appropriate times.

The concept of profiling depth was not used - each Sampler would
only ever have a sampling depth of 1. We still need to call
SignalHandler::IncreaseSamplerCount(), so we do that in Start
and the corresponding DecreaseSamplerCount() in Stop.

Change-Id: I16a9435d26169a7dd00b1c7876e66af45f12e4b0
Reviewed-on: https://chromium-review.googlesource.com/c/1424337
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58955}
2019-01-21 11:45:12 +00:00
Peter Marshall
ba56557793 [cpu-profiler] Cleanup and use std atomics in Sampler
There's no reason to use our self-baked atomics anymore. Also

- Changes two boolean values to use a boolean instead of an int
- Uses a unique ptr for data_
- Removes has_processing_thread_ which is not used
- Moves most initialization inline into the class
- Removes SetUp/TearDown which weren't needed

Change-Id: I8f50133636961502d56351abd2fb17196603a01a
Reviewed-on: https://chromium-review.googlesource.com/c/1422918
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58950}
2019-01-21 10:35:11 +00:00
Peter Marshall
5aa361ffca [cpu-profiler] Add tests for sampler.cc
Moved class definitions into header

Change-Id: I2d3e5ec6f8f5068284cdbaa6900797950fc7e01a
Reviewed-on: https://chromium-review.googlesource.com/c/1422739
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58946}
2019-01-21 09:32:46 +00:00
Florian Sattler
1edbf16697 [cleanup] Refactor general tests to use default members.
Fixing clang-tidy warning.

Bug: v8:8015
Change-Id: I4236a2cf85a414f9d7d1fbdaaaaf1c72a84f02e3
Reviewed-on: https://chromium-review.googlesource.com/1224093
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Florian Sattler <sattlerf@google.com>
Cr-Commit-Position: refs/heads/master@{#55912}
2018-09-14 14:40:47 +00:00
marja
8e7241fdde Include only stuff you need, part 6: Fix cctest.h.
Rebuilding (after touching certain files) is crazy slow because
includes are out of control. Many of these files we need to rebuild are
cctests which pull in more includes than they need.

BUG=v8:5294

Review-Url: https://codereview.chromium.org/2304553002
Cr-Commit-Position: refs/heads/master@{#39080}
2016-09-01 12:02:16 +00:00
lpy
c72f637c73 Move SimulatorHelper into V8 out of profiler clients.
This patch is based on alph's CL https://codereview.chromium.org/2128613004/.

This patch makes GetStackSample propogate the register state when using
simulator helper, and adds argument to avoid using register state from simulator
when pass the native register state.

BUG=v8:4789
LOG=N

Review-Url: https://codereview.chromium.org/2189513002
Cr-Commit-Position: refs/heads/master@{#38554}
2016-08-10 17:12:27 +00:00
machenbach
4af2bb9e38 Revert three commits due to cpu-profiler failures.
Revert "Move SimulatorHelper into V8 out of profiler clients."

This reverts commit b837241150.

Revert "Make use of v8::TickSample instead of v8::internal::TickSample in logger."

This reverts commit c3a16f0a9f.

Revert "Clean up SimulatorHelper code."

This reverts commit 8ee236e144.

BUG=v8:5193
TBR=alph@chromium.org, yangguo@chromium.org

Review-Url: https://codereview.chromium.org/2138643003
Cr-Commit-Position: refs/heads/master@{#37626}
2016-07-11 08:03:26 +00:00
alph
b837241150 Move SimulatorHelper into V8 out of profiler clients.
Clients should not know about the simulator.

BUG=v8:4789

Review-Url: https://codereview.chromium.org/2128613004
Cr-Commit-Position: refs/heads/master@{#37617}
2016-07-08 23:52:49 +00:00
lpy
4e53b1a1a7 Remove v8- prefix of file name in libsampler.
BUG=

Review-Url: https://codereview.chromium.org/2125023004
Cr-Commit-Position: refs/heads/master@{#37599}
2016-07-08 06:47:54 +00:00
lpy
a0198c0f62 Reland: Create libsampler as V8 sampler library.
This patch does five things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Makes SampleStack virtual so embedders can override the behaviour of sample collecting.
3. Removes sampler.[h|cc].
4. Moves sampling thread into log.cc as workaround to keep the --prof functionality.
5. Creates SamplerManager to manage the relationship between samplers and threads.

The reason we port hashmap.h is that in debug mode, STL containers are using
mutexes from a mutex pool, which may lead to deadlock when using asynchronously
signal handler.

Currently libsampler is used in V8 temporarily.

BUG=v8:4789
LOG=n

Committed: https://crrev.com/06cc9b7c176a6223971deaa9fbcafe1a05058c7b
Cr-Commit-Position: refs/heads/master@{#36527}

Review-Url: https://codereview.chromium.org/1922303002
Cr-Commit-Position: refs/heads/master@{#36532}
2016-05-26 02:14:50 +00:00
lpy
636f1e8e59 Revert of Create libsampler as V8 sampler library. (patchset #24 id:460001 of https://codereview.chromium.org/1922303002/ )
Reason for revert:
V8 Linux64 TSAN failure because ThreadSanitizer indicated data race.

Original issue's description:
> Create libsampler as V8 sampler library.
>
> This patch does five things:
>
> 1. Extracts sampler as libsampler to provide sampling functionality support.
> 2. Makes SampleStack virtual so embedders can override the behaviour of sample collecting.
> 3. Removes sampler.[h|cc].
> 4. Moves sampling thread into log.cc as workaround to keep the --prof functionality.
> 5. Creates SamplerManager to manage the relationship between samplers and threads.
>
> The reason we port hashmap.h is that in debug mode, STL containers are using
> mutexes from a mutex pool, which may lead to deadlock when using asynchronously
> signal handler.
>
> Currently libsampler is used in V8 temporarily.
>
> BUG=v8:4789
> LOG=n
>
> Committed: https://crrev.com/06cc9b7c176a6223971deaa9fbcafe1a05058c7b
> Cr-Commit-Position: refs/heads/master@{#36527}

TBR=jochen@chromium.org,alph@chromium.org,fmeawad@chromium.org,yangguo@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:4789

Review-Url: https://codereview.chromium.org/2000323007
Cr-Commit-Position: refs/heads/master@{#36529}
2016-05-25 20:23:33 +00:00
lpy
06cc9b7c17 Create libsampler as V8 sampler library.
This patch does five things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Makes SampleStack virtual so embedders can override the behaviour of sample collecting.
3. Removes sampler.[h|cc].
4. Moves sampling thread into log.cc as workaround to keep the --prof functionality.
5. Creates SamplerManager to manage the relationship between samplers and threads.

The reason we port hashmap.h is that in debug mode, STL containers are using
mutexes from a mutex pool, which may lead to deadlock when using asynchronously
signal handler.

Currently libsampler is used in V8 temporarily.

BUG=v8:4789
LOG=n

Review-Url: https://codereview.chromium.org/1922303002
Cr-Commit-Position: refs/heads/master@{#36527}
2016-05-25 19:06:45 +00:00