1. 11 Feb, 2019 1 commit
  2. 21 Jan, 2019 1 commit
    • Peter Marshall's avatar
      [logger] Start cleaning up Logger class · 7da7c0bd
      Peter Marshall authored
      - Use unique ptrs for owned objects
      - Remove friendship with CpuProfiler and replace with public API
      - Remove unused method LogFailure()
      - Remove StopProfiler() which was only used by LogFailure() (removed)
        and one test, which can use StopProfilerThread() instead
      - Remove 'paused' state which was only used by the above
      - Remove 'engage' state. There is no reason we need this as along as
        users keep track of Engage/Disengage calls
      
      Drive-by cleanup:
      - Remove import of log.h from profile-generator.h
      - Remove unnecessary includes of log.h
      
      Change-Id: Ifc4ca156bef038c40953f8361ffea17788e3a59b
      Reviewed-on: https://chromium-review.googlesource.com/c/1424338
      Commit-Queue: Peter Marshall <petermarshall@chromium.org>
      Reviewed-by: 's avatarMichael Starzinger <mstarzinger@chromium.org>
      Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#58957}
      7da7c0bd
  3. 15 Jan, 2019 2 commits
  4. 14 Jan, 2019 2 commits
  5. 08 Jan, 2019 1 commit
    • Peter Marshall's avatar
      [cpu-profiler] De-duplicate CodeEntry objects for inline stacks · 9e158605
      Peter Marshall authored
      Within an inline stack we would have multiple copies of the exact same
      CodeEntry object to represent an inline frame. We had one copy for every
      time that the frame appeared in an inline stack. One CodeEntry can have
      multiple inline stacks and each stack can have multiple inline frames.
      In the common case, the stacks overlap and repeat frames.
      
      This CL creates a single CodeEntry object to represent each inlined
      function as an inline frame (for a given CodeEntry with inlinings). This
      removes most of the duplication of inline CodeEntry objects. We still
      have some duplication, e.g. if we inline bar() into foo() and foo2() but
      they are not themselves inlined into anything, then we will have two
      inline CodeEntry objects for bar(). Removing all duplication is harder
      to achieve because the lifetime of the inlined frame CodeEntry is now no
      longer tied to the inliner.
      
      Get rid of the InlineEntry struct as it is now indentical to
      CodeEntryAndLineNumber.
      
      We store the list of canonical inline CodeEntry objects on the
      CodeObject of the inlining function so that it can own the lifetimes of
      inlined frames.
      
      Also rename inline_locations_ to inline_stacks_ to be clearer.
      
      Bug: v8:7719
      
      Change-Id: Ied765b4cce7fd33f3290798331f1e6767cc42e8c
      Reviewed-on: https://chromium-review.googlesource.com/c/1396086
      Commit-Queue: Peter Marshall <petermarshall@chromium.org>
      Reviewed-by: 's avatarTobias Tebbi <tebbi@chromium.org>
      Reviewed-by: 's avatarAlexei Filippov <alph@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#58639}
      9e158605
  6. 04 Jan, 2019 2 commits
    • Peter Marshall's avatar
      [cpu-profiler] Reduce the size of inlining information · a0572f0b
      Peter Marshall authored
      Previously we stored the source position table, which stored a mapping
      of pc offsets to line numbers, and the inline_locations, which stored a
      mapping of pc offsets to stacks of {CodeEntry, line_number} pairs. This
      was slightly wasteful because we had two different tables which were
      both keyed on the pc offset and contained some overlapping information.
      
      This CL combines the two tables in a way. The source position table now
      maps a pc offset to a pair of {line_number, inlining_id}. If the
      inlining_id is valid, then it can be used to look up the inlining stack
      which is stored in inline_locations, but is now keyed by inlining_id
      rather than pc offset. This also has the nice effect of de-duplicating
      inline stacks which we previously duplicated.
      
      The new structure is similar to how this data is stored by the compiler,
      except that we convert 'source positions' (char offset in a file) into
      line numbers as we go, because we only care about attributing ticks to
      a given line.
      
      Also remove the helper RecordInliningInfo() as this is only actually
      used to add inline stacks by one caller (where it is now inlined). The
      other callers would always bail out or are only called from
      test-cpu-profiler.
      
      Remove AddInlineStack and replace it with SetInlineStacks which adds all
      of the stacks at once. We need to do it this way because the source pos
      table is passed into the constructor of CodeEntry, so we need to create
      it before the CodeEntry, but the inline stacks are not (they are part of
      rare_data which is not always present), so we need to add them after
      construction. Given that we calculate both the source pos table and the
      inline stacks before construction, it's just easier to add them all at
      once.
      
      Also add a print() method to CodeEntry to make future debugging easier
      as I'm constantly rewriting this locally.
      
      Bug: v8:8575, v8:7719, v8:7203
      
      Change-Id: I39324d6ea13d116d5da5d0a0d243cae76a749c79
      Reviewed-on: https://chromium-review.googlesource.com/c/1392195
      Commit-Queue: Peter Marshall <petermarshall@chromium.org>
      Reviewed-by: 's avatarTobias Tebbi <tebbi@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#58554}
      a0572f0b
    • Peter Marshall's avatar
      [cpu-profiler] Add source positions for inlined function calls · af0428ac
      Peter Marshall authored
      Currently in both kCallerLineNumbers and kLeafNodeLineNumbers modes, we
      correctly capture inline stacks. In leaf number mode, this is simple as
      we simply add the path onto the existing tree. For caller line numbers
      mode this is more complex, because each path through various inlined
      function should be represented in the tree, even when there are
      multiple callsites to the same function inlined.
      
      Currently we don't correctly show line numbers for inlined functions.
      We do actually have this information though, which is generated by
      turbofan and stored in the source_position_table data structure on the
      code object.
      
      This also changes the behavior of the SourcePositionTable class. A
      problem we uncovered is that the PC that the sampler provides for every
      frame except the leaf is the return address of the calling frame. This
      address is *after* the call has already happened. It can be attributed
      to the next line of the function, rather than the calling line, which
      is wrong. We fix that here by using lower_bound in GetSourceLineNumber.
      
      The same problem happens in GetInlineStack - the PC of the caller is
      actually the instruction after the call. The information turbofan
      generates assumes that the instruction after the call is not part of
      the call (fair enough). To fix this we do the same thing as above - use
      lower_bound and then iterate back by one.
      
      TBR=alph@chromium.org
      
      Bug: v8:8575, v8:8606
      Change-Id: Idc4bd4bdc8fb70b70ecc1a77a1e3744a86f83483
      Reviewed-on: https://chromium-review.googlesource.com/c/1374290
      Commit-Queue: Peter Marshall <petermarshall@chromium.org>
      Reviewed-by: 's avatarTobias Tebbi <tebbi@chromium.org>
      Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#58545}
      af0428ac
  7. 28 Nov, 2018 1 commit
  8. 27 Nov, 2018 1 commit
  9. 20 Sep, 2018 1 commit
  10. 27 Jul, 2018 1 commit
    • Peter Marshall's avatar
      [cpu-profiler] Use instruction start as the key for the CodeMap · ba752ea4
      Peter Marshall authored
      Previously we used the start address of the AbstractCode object. This
      doesn't make sense for off-heap builtins, where the code isn't contained
      in the object itself. It also hides other potential problems - sometimes
      the sample.pc is inside the AbstractCode object header - this is
      never valid.
      
      There were a few changes necessary to make this happen:
        - Change the interface of CodeMoveEvent. Now 'to' and 'from' are both
          AbstractCode objects, which is nice because many users were taking
          'to' and adding the header offset to it to try and find the
          instruction start address. This isn't valid for off-heap builtins.
        - Fix a bug in CodeMap::MoveCode where we didn't update the CodeEntry
          object to reflect the new instruction_start.
        - Rename the 'start' field in all of the CodeEventRecord sub-classes
          to make it clear that this is the address of the first instruction.
        - Fix the confusion in RecordTickSample between 'tos' and 'pc' which
          caused pc_offset to be calculated incorrectly.
      
      Bug: v8:7983
      Change-Id: I3e9dddf74e4b2e96a5f031d216ef7008d6f184d1
      Reviewed-on: https://chromium-review.googlesource.com/1148457
      Commit-Queue: Peter Marshall <petermarshall@chromium.org>
      Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
      Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#54749}
      ba752ea4
  11. 03 Jul, 2018 1 commit
  12. 24 May, 2018 1 commit
    • Peter Marshall's avatar
      [cpu-profiler] Only store deopt inline frames for functions that need it · 0bfcbdd4
      Peter Marshall authored
      We store deopt inline frames for all functions when we receive the code
      creation event. We only ever use this information for code which is
      deoptimized. Given that we receive code deopt events, we can just store
      this information when the code is deoptimized.
      
      At the time of the code deopt event, we also know the associated
      deopt_id. That means we don't need to store a map of deopt_ids to
      vectors of frames, because we will only ever access the frames for the
      deopt_id that is already set.
      
      This means we store way less data, particularly for long-running
      processes which see fewer deopts. This saves 10MiB peak memory on the
      node server example.
      
      Bug: v8:7719
      Change-Id: If6cf5ec413848e4c9f3c1e2106366ae2adae6fb1
      Reviewed-on: https://chromium-review.googlesource.com/1050289
      Commit-Queue: Peter Marshall <petermarshall@chromium.org>
      Reviewed-by: 's avatarAlexei Filippov <alph@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#53330}
      0bfcbdd4
  13. 23 May, 2018 3 commits
    • Alexei Filippov's avatar
      [cpu-profiler] Reuse free slots in code_entries_ · 3e1126bf
      Alexei Filippov authored
      The patch makes it manage a free list of released code_entries_ slots,
      and reuse the slots as needed.
      
      BUG=v8:7719
      
      Change-Id: I07df1ce983fe00e0ca3d1a1ea20e1a141aabad99
      Reviewed-on: https://chromium-review.googlesource.com/1062769Reviewed-by: 's avatarPeter Marshall <petermarshall@chromium.org>
      Commit-Queue: Alexei Filippov <alph@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#53314}
      3e1126bf
    • Alexei Filippov's avatar
      [cpu-profiler] Prefix wasm resource names with "wasm " · 1143a6c7
      Alexei Filippov authored
      BUG=chromium:844150
      
      Change-Id: I0f7e10fb9778b3de76591ad4819be45c8c50c8d4
      Reviewed-on: https://chromium-review.googlesource.com/1064815Reviewed-by: 's avatarStephan Herhut <herhut@chromium.org>
      Commit-Queue: Alexei Filippov <alph@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#53313}
      1143a6c7
    • Peter Marshall's avatar
      [cpu-profiler] Add a new profiling mode with a more detailed call tree. · ecae80cd
      Peter Marshall authored
      The current profiling mode (called kLeafNodeLineNumbers in this CL)
      produces a tree, with each node representing a stack frame that is seen
      in one or more samples taken during profiling. These nodes refer to a
      particular function in a stack trace, but not to a particular line or
      callsite within that function.
      
      This CL adds a new more (called kCallerLineNumbers) which produces a
      different profile tree, where each stack trace seen during profiling,
      including the line number, has a unique path in the tree.
      
      The profile tree was previously keyed on CodeEntry*. Now it is keyed on
      the pair of CodeEntry* and line_number, meaning it has distinct nodes
      for those combinations which exist, and each distinct stack trace that
      was sampled is represented in the tree.
      
      For optimized code where we have inline frames, there are no line
      numbers for the inline frames in the stack trace, causing duplicate
      branches in the tree with kNoLineNumberInfo as the reported line number.
      This will be addressed in follow-ups.
      
      Bug: v8:7018
      Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
      Change-Id: I512e221508f5b50ec028306d212263b514a9fb24
      Reviewed-on: https://chromium-review.googlesource.com/1013493
      Commit-Queue: Peter Marshall <petermarshall@chromium.org>
      Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#53298}
      ecae80cd
  14. 22 May, 2018 1 commit
  15. 18 May, 2018 1 commit
    • Peter Marshall's avatar
      [cpu-profiler] Move bailout reason into rare_info struct · 29ea4d1e
      Peter Marshall authored
      This was set very regularly in FillFunctionInfo, but it was almost
      always set to kNoReason, because the associated SFI had no bailout
      reason. Given that having a bailout reason is the rare case, we
      just assume an empty bailout reason, and use the rare_data_ struct
      to store the string pointer if we do need it.
      
      This saves another pointer of space on the CodeEntry object (approx
      1.4 MiB on the node server example).
      
      Bug: v8:7719
      Change-Id: I8e2272b572285ddf353ba0b303e6da095b7d5272
      Reviewed-on: https://chromium-review.googlesource.com/1064370
      Commit-Queue: Peter Marshall <petermarshall@chromium.org>
      Reviewed-by: 's avatarAlexei Filippov <alph@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#53244}
      29ea4d1e
  16. 16 May, 2018 1 commit
    • Alexei Filippov's avatar
      [cpu-profiler] Eagerly delete not used CodeEntry'es · c6c28f7a
      Alexei Filippov authored
      Currently ProfilerListener holds all the CodeEntries it ever
      created during the profiling session. It is not capable of removing
      entries corresponding to the code objects discarded by GC as there's
      no such code event.
      
      However it is sometimes possible to tell if a code object was GCed.
      Hook up to the CodeMap code entry removal and if the entry has never
      been hit by a sample we can safely delete it.
      
      As a bonus the CodeEntryInfo size has been reduced on x64, which also
      saves 8 x <number of code entries> bytes.
      
      BUG=v8:7719
      
      Change-Id: I988bc5b59f3fba07157a9f472cbcf68596fcd969
      Reviewed-on: https://chromium-review.googlesource.com/1054346Reviewed-by: 's avatarPeter Marshall <petermarshall@chromium.org>
      Commit-Queue: Alexei Filippov <alph@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#53222}
      c6c28f7a
  17. 11 May, 2018 1 commit
  18. 07 May, 2018 2 commits
  19. 04 May, 2018 1 commit
  20. 23 Apr, 2018 1 commit
  21. 14 Apr, 2018 1 commit
    • Jakob Kummerow's avatar
      [ubsan] Change Address typedef to uintptr_t · 2459046c
      Jakob Kummerow authored
      The "Address" type is V8's general-purpose type for manipulating memory
      addresses. Per the C++ spec, pointer arithmetic and pointer comparisons
      are undefined behavior except within the same array; since we generally
      don't operate within a C++ array, our general-purpose type shouldn't be
      a pointer type.
      
      Bug: v8:3770
      Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng;master.tryserver.blink:linux_trusty_blink_rel
      Change-Id: Ib96016c24a0f18bcdba916dabd83e3f24a1b5779
      Reviewed-on: https://chromium-review.googlesource.com/988657
      Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
      Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#52601}
      2459046c
  22. 12 Apr, 2018 1 commit
    • Peter Marshall's avatar
      [cpu-profiler] Fix bugs and add tests for JITLineInfoTable · 4feb5ce7
      Peter Marshall authored
      Looking up line numbers with the JITLineInfoTable would sometimes give
      wrong answers. Fix these bugs and add a cctest for this data structure.
      
      Also do some cleanup while we're here like inlining the (empty)
      constructor and destructor and removing the empty() method which is
      only used unnecessarily anyway, to make the contract of
      GetSourceLineNumber a bit clearer.
      
      Also rename the data structure to SourcePositionTable, because it
      doesn't just provide info for JIT code, but also bytecode, and 'Info'
      is pretty ambiguous.
      
      Bug: v8:7018
      Change-Id: I126581c844d85df6b2b3f80f2f5acbce01c16ba1
      Reviewed-on: https://chromium-review.googlesource.com/1006795Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Commit-Queue: Peter Marshall <petermarshall@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#52571}
      4feb5ce7
  23. 23 Feb, 2018 1 commit
  24. 06 Feb, 2018 1 commit
  25. 05 Feb, 2018 1 commit
  26. 02 Feb, 2018 1 commit
  27. 13 Oct, 2017 1 commit
  28. 07 Sep, 2017 1 commit
  29. 29 Aug, 2017 1 commit
  30. 27 Jan, 2017 1 commit
  31. 19 Dec, 2016 2 commits
  32. 22 Nov, 2016 1 commit
    • tebbi's avatar
      [cpu-profiler] use new source position information for deoptimization in cpu profiler · 1b320d20
      tebbi authored
      The new SourcePosition class allows for precise tracking of source positions including the stack of inlinings. This CL makes the cpu profiler use this new information. Before, the cpu profiler used the deoptimization data to reconstruct the inlining stack. However, optimizing compilers (especially Turbofan) can hoist out checks such that the inlining stack of the deopt reason and the inlining stack of the position the deoptimizer jumps to can be different (the old cpu profiler tests and the ones introduced in this cl produce such situations for turbofan). In this case, relying on the deoptimization info produces paradoxical results, where the reported position is before the function responsible is called. Even worse, https://codereview.chromium.org/2451853002/ combines the precise position with the wrong inlining stack from the deopt info, leading to completely wrong results.
      
      Other changes in this CL:
      - DeoptInlinedFrame is no longer needed, because we can compute the correct inlining stack up front.
      - I changed the cpu profiler tests back to test situations where deopt checks are hoisted out in Turbofan and made them robust enough to handle the differences between Crankshaft and Turbofan.
      - I reversed the order of SourcePosition::InliningStack to make it match the cpu profiler convention.
      - I removed CodeDeoptEvent::position, as it is no longer used.
      
      R=alph@chromium.org
      
      BUG=v8:5432
      
      Review-Url: https://codereview.chromium.org/2503393002
      Cr-Commit-Position: refs/heads/master@{#41168}
      1b320d20
  33. 28 Oct, 2016 1 commit