1. 09 Feb, 2021 1 commit
  2. 03 Dec, 2020 1 commit
  3. 25 Nov, 2020 1 commit
  4. 28 Oct, 2020 1 commit
  5. 20 Oct, 2020 1 commit
  6. 16 Sep, 2020 1 commit
    • Alex Kodat's avatar
      [cpu-profiler] Ensure sampled thread has Isolate lock under Windows · 76217f57
      Alex Kodat authored
      While the sampler checked if the sampled thread had the Isolate locked
      (if locks are being used) under Linux, the check was not done under
      Windows (or Fuchsia) which meant that in a multi-threading application
      under Windows, thread locking was not checked making it prone to seg
      faults and the like as the profiler would be using isolate->js_entry_sp
      to determine the stack to walk but isolate->js_entry_sp is the stack
      pointer for the thread that currently has the Isolate lock so, if the
      sampled thread does not have the lock, the sampler woud be iterating
      over the wrong stack, one that might actually be actively changing on
      another thread. The fix was to move the lock check into CpuSampler
      and Ticker (--prof) so all OSes would do the correct check.
      
      The basic concept is that on all operating systems a CpuProfiler, and
      so its corresponding CpuCampler, the profiler is tied to a thread.
      This is not based on first principles or anything, it's simply the
      way it works in V8, though it is a useful conceit as it makes
      visualization and interpretation of profile data much easier.
      
      To collect a sample on a thread associated with a profiler the thread
      must be stopped for obvious reasons -- walking the stack of a running
      thread is a formula for disaster. The mechanism for stopping a thread
      is OS-specific and is done in sample.cc. There are currently three
      basic approaches, one for Linux/Unix variants, one for Windows and one
      for Fuchsia. The approaches vary as to which thread actually collects
      the sample -- under Linux the sample is actually collected on the
      (interrupted) sampled thread whereas under Fuchsia/Windows it's on
      a separate thread.
      
      However, in a multi-threaded environment (where Locker is used), it's
      not sufficient for the sampled thread to be stopped. Because the stack
      walk involves looking in the Isolate heap, no other thread can be
      messing with the heap while the sample is collected. The only ways to
      ensure this would be to either stop all threads whenever collecting a
      sample, or to ensure that the thread being sampled holds the Isolate
      lock so prevents other threads from messing with the heap. While there
      might be something to be said for the "stop all threads" approach, the
      current approach in V8 is to only stop the sampled thread so, if in a
      multi-threaded environment, the profiler must check if the thread being
      sampled holds the Isolate lock.
      
      Since this check must be done, independent of which thread the sample
      is being collected on (since it varies from OS to OS), the approach is
      to save the thread id of the thread to be profiled/sampled when the
      CpuSampler is instantiated (on all OSes it is instantiated on the
      sampled thread) and then check that thread id against the Isolate lock
      holder thread id before collecting a sample. If it matches, we know
      sample.cc has stop the sampled thread, one way or another, and we know
      that no other thread can mess with the heap (since the stopped thread
      holds the Isolate lock) so it's safe to walk the stack and collect data
      from the heap so the sample can be taken. It it doesn't match, we can't
      safely collect the sample so we don't.
      
      Bug: v8:10850
      Change-Id: Iba6cabcd3e11a19c261c004103e37e806934dc6f
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2411343Reviewed-by: 's avatarPeter Marshall <petermarshall@chromium.org>
      Commit-Queue: Peter Marshall <petermarshall@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#69952}
      76217f57
  7. 01 Sep, 2020 2 commits
    • Peter Marshall's avatar
      [cpu-profiler] Add stats to track missing or unnattributed frames · ca6675ed
      Peter Marshall authored
      This adds a global counter for the various reasons we might fail to
      attribute a tick.
      
      The counters are cleared and printed when Profile::Print() is called,
      which we call in our tests, so flaky test output will now contain these
      stats along with the printed profile tree.
      
      Drive-by cleanup some print functions and make them const.
      
      Change-Id: Ia3a27405f5b5346adfdbb32afc7e414857969cc5
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1550406
      Commit-Queue: Peter Marshall <petermarshall@chromium.org>
      Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#69647}
      ca6675ed
    • Peter Marshall's avatar
      Revert "[cpu-profiler] Ensure sampled thread has Isolate lock under Windows" · 32435062
      Peter Marshall authored
      This reverts commit dfb3f7da.
      
      Reason for revert: Breaks LSAN & ASAN flakily: https://bugs.chromium.org/p/v8/issues/detail?id=10861
      
      Original change's description:
      > [cpu-profiler] Ensure sampled thread has Isolate lock under Windows
      > 
      > While the sampler checked if the sampled thread had the Isolate locked
      > (if locks are being used) under Linux, the check was not done under
      > Windows (or Fuchsia) which meant that in a multi-threading application
      > under Windows, thread locking was not checked making it prone to seg
      > faults and the like as the profiler would be extracting info from a
      > heap in motion. The fix was to move the lock check into CpuSampler
      > and Ticker (--prof) so all OSes would do the correct check.
      > 
      > The basic concept is that on all operating systems a CpuProfiler, and
      > so its corresponding CpuCampler, the profiler is tied to a thread.
      > This is not based on first principles or anything, it's simply the
      > way it works in V8, though it is a useful conceit as it makes
      > visualization and interpretation of profile data much easier.
      > 
      > To collect a sample on a thread associated with a profiler the thread
      > must be stopped for obvious reasons -- walking the stack of a running
      > thread is a formula for disaster. The mechanism for stopping a thread
      > is OS-specific and is done in sample.cc. There are currently three
      > basic approaches, one for Linux/Unix variants, one for Windows and one
      > for Fuchsia. The approaches vary as to which thread actually collects
      > the sample -- under Linux the sample is actually collected on the
      > (interrupted) sampled thread whereas under Fuchsia/Windows it's on
      > a separate thread.
      > 
      > However, in a multi-threaded environment (where Locker is used), it's
      > not sufficient for the sampled thread to be stopped. Because the stack
      > walk involves looking in the Isolate heap, no other thread can be
      > messing with the heap while the sample is collected. The only ways to
      > ensure this would be to either stop all threads whenever collecting a
      > sample, or to ensure that the thread being sampled holds the Isolate
      > lock so prevents other threads from messing with the heap. While there
      > might be something to be said for the "stop all threads" approach, the
      > current approach in V8 is to only stop the sampled thread so, if in a
      > multi-threaded environment, the profiler must check if the thread being
      > sampled holds the Isolate lock.
      > 
      > Since this check must be done, independent of which thread the sample
      > is being collected on (since it varies from OS to OS), the approach is
      > to save the thread id of the thread to be profiled/sampled when the
      > CpuSampler is instantiated (on all OSes it is instantiated on the
      > sampled thread) and then check that thread id against the Isolate lock
      > holder thread id before collecting a sample. If it matches, we know
      > sample.cc has stop the sampled thread, one way or another, and we know
      > that no other thread can mess with the heap (since the stopped thread
      > holds the Isolate lock) so it's safe to walk the stack and collect data
      > from the heap so the sample can be taken. It it doesn't match, we can't
      > safely collect the sample so we don't.
      > 
      > Bug: v8:10850
      > Change-Id: Iab2493130b9328430d7e5f5d3cf90ad6d10b1892
      > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2377108
      > Reviewed-by: Peter Marshall <petermarshall@chromium.org>
      > Commit-Queue: Peter Marshall <petermarshall@chromium.org>
      > Cr-Commit-Position: refs/heads/master@{#69623}
      
      TBR=akodat@rocketsoftware.com,petermarshall@chromium.org,petermarshall@google.com
      
      Change-Id: Ib6b6dc4ce109d5aa4e504fa7c9769f5cd95ddd0c
      No-Presubmit: true
      No-Tree-Checks: true
      No-Try: true
      Bug: v8:10850
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2387570Reviewed-by: 's avatarPeter Marshall <petermarshall@chromium.org>
      Commit-Queue: Peter Marshall <petermarshall@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#69638}
      32435062
  8. 31 Aug, 2020 1 commit
    • Alex Kodat's avatar
      [cpu-profiler] Ensure sampled thread has Isolate lock under Windows · dfb3f7da
      Alex Kodat authored
      While the sampler checked if the sampled thread had the Isolate locked
      (if locks are being used) under Linux, the check was not done under
      Windows (or Fuchsia) which meant that in a multi-threading application
      under Windows, thread locking was not checked making it prone to seg
      faults and the like as the profiler would be extracting info from a
      heap in motion. The fix was to move the lock check into CpuSampler
      and Ticker (--prof) so all OSes would do the correct check.
      
      The basic concept is that on all operating systems a CpuProfiler, and
      so its corresponding CpuCampler, the profiler is tied to a thread.
      This is not based on first principles or anything, it's simply the
      way it works in V8, though it is a useful conceit as it makes
      visualization and interpretation of profile data much easier.
      
      To collect a sample on a thread associated with a profiler the thread
      must be stopped for obvious reasons -- walking the stack of a running
      thread is a formula for disaster. The mechanism for stopping a thread
      is OS-specific and is done in sample.cc. There are currently three
      basic approaches, one for Linux/Unix variants, one for Windows and one
      for Fuchsia. The approaches vary as to which thread actually collects
      the sample -- under Linux the sample is actually collected on the
      (interrupted) sampled thread whereas under Fuchsia/Windows it's on
      a separate thread.
      
      However, in a multi-threaded environment (where Locker is used), it's
      not sufficient for the sampled thread to be stopped. Because the stack
      walk involves looking in the Isolate heap, no other thread can be
      messing with the heap while the sample is collected. The only ways to
      ensure this would be to either stop all threads whenever collecting a
      sample, or to ensure that the thread being sampled holds the Isolate
      lock so prevents other threads from messing with the heap. While there
      might be something to be said for the "stop all threads" approach, the
      current approach in V8 is to only stop the sampled thread so, if in a
      multi-threaded environment, the profiler must check if the thread being
      sampled holds the Isolate lock.
      
      Since this check must be done, independent of which thread the sample
      is being collected on (since it varies from OS to OS), the approach is
      to save the thread id of the thread to be profiled/sampled when the
      CpuSampler is instantiated (on all OSes it is instantiated on the
      sampled thread) and then check that thread id against the Isolate lock
      holder thread id before collecting a sample. If it matches, we know
      sample.cc has stop the sampled thread, one way or another, and we know
      that no other thread can mess with the heap (since the stopped thread
      holds the Isolate lock) so it's safe to walk the stack and collect data
      from the heap so the sample can be taken. It it doesn't match, we can't
      safely collect the sample so we don't.
      
      Bug: v8:10850
      Change-Id: Iab2493130b9328430d7e5f5d3cf90ad6d10b1892
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2377108Reviewed-by: 's avatarPeter Marshall <petermarshall@chromium.org>
      Commit-Queue: Peter Marshall <petermarshall@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#69623}
      dfb3f7da
  9. 29 Jun, 2020 1 commit
  10. 09 Jun, 2020 1 commit
  11. 21 Feb, 2020 1 commit
  12. 28 Nov, 2019 1 commit
  13. 13 Sep, 2019 1 commit
  14. 10 Sep, 2019 1 commit
  15. 30 May, 2019 1 commit
  16. 28 May, 2019 1 commit
  17. 27 May, 2019 1 commit
    • Clemens Hammacher's avatar
      [cleanup] Replace simple typedefs by using · a335f2ae
      Clemens Hammacher authored
      This replaces all typedefs that define types and not functions by the
      equivalent "using" declaration.
      
      This was done mostly automatically using this command:
      ag -l '\btypedef\b' src test | xargs -L1 \
           perl -i -p0e 's/typedef ([^*;{}]+) (\w+);/using \2 = \1;/sg'
      
      Patchset 2 then adds some manual changes for typedefs for pointer types,
      where the regular expression did not match.
      
      R=mstarzinger@chromium.org
      TBR=yangguo@chromium.org, jarin@chromium.org
      
      Bug: v8:9183
      Change-Id: I6f6ee28d1793b7ac34a58f980b94babc21874b78
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1631409
      Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
      Reviewed-by: 's avatarMichael Starzinger <mstarzinger@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#61849}
      a335f2ae
  18. 17 May, 2019 1 commit
  19. 02 May, 2019 1 commit
  20. 24 Apr, 2019 1 commit
  21. 17 Apr, 2019 2 commits
  22. 27 Mar, 2019 1 commit
  23. 14 Feb, 2019 1 commit
  24. 13 Feb, 2019 1 commit
  25. 21 Jan, 2019 4 commits
    • Peter Marshall's avatar
      [cpu-profiler] Remove registration and sampling depth from Sampler · 1f1bd71d
      Peter Marshall authored
      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: 's avatarJakob Gruber <jgruber@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#58955}
      1f1bd71d
    • Peter Marshall's avatar
      [cpu-profiler] Cleanup and use std atomics in Sampler · ba565577
      Peter Marshall authored
      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: 's avatarJakob Gruber <jgruber@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#58950}
      ba565577
    • Peter Marshall's avatar
      [cpu-profiler] Add tests for sampler.cc · 5aa361ff
      Peter Marshall authored
      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: 's avatarJakob Gruber <jgruber@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#58946}
      5aa361ff
    • Peter Marshall's avatar
      [cpu-profiler] Use unordered_map instead of custom hashmap in sampler · 4dbdeea2
      Peter Marshall authored
      There is no reason to use the custom map here anymore. This lets us
      get rid of the custom hash and a lot of casts.
      
      We can also store the SamplerList by value in the map rather than a
      pointer, then we don't have to manage the lifetime explicitly.
      
      Also move the SamplerList typedef inside of SamplerManager because it's
      an internal detail. Remove the include for <map> because we aren't using
      this anywhere anyway.
      
      Change-Id: I787a1b6c3ffc331ec3f36e66d5e07bd115c4cbb4
      Reviewed-on: https://chromium-review.googlesource.com/c/1419317Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Commit-Queue: Peter Marshall <petermarshall@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#58945}
      4dbdeea2
  26. 18 Jan, 2019 2 commits
  27. 16 Jan, 2019 1 commit
  28. 15 Jan, 2019 2 commits
  29. 14 Jan, 2019 2 commits
  30. 02 Jan, 2019 1 commit
  31. 25 Oct, 2018 1 commit
    • Peter Marshall's avatar
      Reland "[cpu-profiler] Fix a bug which caused a pure virtual function call" · 3767ab39
      Peter Marshall authored
      This is a reland of c92a1dda
      
      Original change's description:
      > [cpu-profiler] Fix a bug which caused a pure virtual function call
      >
      > We need to remove each Sampler from the SamplerManager before we call
      > the Sampler destructor. This is because the signal handler can interrupt
      > the destructor, and call DoSampler(), which calls sampler->SampleStack()
      > on the sampler being destructed, causing general unhappiness and
      > "Pure virtual function called!" crashes.
      >
      > Bug: v8:8346, v8:5193
      > Change-Id: Iaa595a196eab33fb1af31584e9a68fd1ce0a18f6
      > Reviewed-on: https://chromium-review.googlesource.com/c/1293949
      > Commit-Queue: Peter Marshall <petermarshall@chromium.org>
      > Reviewed-by: Alexei Filippov <alph@chromium.org>
      > Cr-Commit-Position: refs/heads/master@{#56882}
      
      TBR=yangguo@chromium.org
      
      Bug: v8:8346, v8:5193
      Change-Id: I9878f65c868ff1aed6f3a587cba688c4241bad8c
      Reviewed-on: https://chromium-review.googlesource.com/c/1298893Reviewed-by: 's avatarPeter Marshall <petermarshall@chromium.org>
      Commit-Queue: Peter Marshall <petermarshall@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#56976}
      3767ab39
  32. 24 Oct, 2018 1 commit