1. 03 Mar, 2021 1 commit
    • Seth Brenith's avatar
      Privatize FixedArray-style accessors on ScopeInfo · 3a366c4d
      Seth Brenith authored
      This is a partial reland of https://crrev.com/c/2601880 .
      
      In preparation for ScopeInfo not being a FixedArrayBase, this change
      privatizes the FixedArray-style functions that provide access to
      ScopeInfo fields by index, and moves them from scope-info-inl.h to
      scope-info.cc. Those functions are still used pretty heavily during
      initialization (ScopeInfo::Create, etc.), but at least we can avoid
      presenting them to the rest of the world.
      
      This change also introduces a new length() function in ScopeInfo which
      hides the one inherited from FixedArrayBase and computes the ScopeInfo's
      length based on its flags, so that there are no remaining readers of the
      'length' field.
      
      Change-Id: I609754010723b679e5cf00f386020faaab84c17a
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2718275
      Commit-Queue: Toon Verwaest <verwaest@chromium.org>
      Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#73151}
      3a366c4d
  2. 19 Feb, 2021 1 commit
    • Seth Brenith's avatar
      Revert "Remove 'length' field from ScopeInfo" · 6c922e39
      Seth Brenith authored
      This reverts commit f731e13f.
      
      Reason for revert: perf regressions, chromium:1179757
      
      Original change's description:
      > Remove 'length' field from ScopeInfo
      >
      > ScopeInfo has a vestigial 'length' field from when it used to be a
      > FixedArray. This change removes that field, which saves some memory.
      >
      > More specifically:
      >
      > - Make ScopeInfo inherit from HeapObject, not FixedArrayBase which
      >   supplied the 'length' field.
      > - Privatize the FixedArray-style functions that provide access to
      >   ScopeInfo fields by index, and move them from scope-info-inl.h to
      >   scope-info.cc. Those functions are still used pretty heavily during
      >   initialization (ScopeInfo::Create, etc.), but at least we can avoid
      >   presenting them to the rest of the world.
      > - Change FactoryBase::NewScopeInfo to allocate the updated object shape.
      >   It maintains the existing behavior of filling the newly-allocated
      >   object with undefined, even though that's not a valid ScopeInfo and
      >   further initialization is required.
      > - Move part of AccessorAssembler::ScriptContextTableLookup into a new
      >   Torque macro, because it used to rely on casting ScopeInfo to
      >   FixedArrayBase.
      > - In V8HeapExplorer::AddEntry, don't claim that ScopeInfo objects are
      >   arrays. I think it makes more sense to list them under "(system)" in
      >   the dev tools, like most other V8 internal types.
      >
      > Bug: v8:8952
      > Change-Id: I8278e3a90027d4409f0d268da0fe7080754c6b8c
      > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2601880
      > Reviewed-by: Toon Verwaest <verwaest@chromium.org>
      > Reviewed-by: Peter Marshall <petermarshall@chromium.org>
      > Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
      > Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
      > Reviewed-by: Mythri Alle <mythria@chromium.org>
      > Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
      > Cr-Commit-Position: refs/heads/master@{#72830}
      
      Bug: v8:8952
      Change-Id: I00a69da79e5ac6aaae4436a41ce773ae014cc775
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2706086
      Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
      Auto-Submit: Seth Brenith <seth.brenith@microsoft.com>
      Commit-Queue: Nico Hartmann <nicohartmann@chromium.org>
      Reviewed-by: 's avatarNico Hartmann <nicohartmann@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#72855}
      6c922e39
  3. 17 Feb, 2021 1 commit
    • Seth Brenith's avatar
      Remove 'length' field from ScopeInfo · f731e13f
      Seth Brenith authored
      ScopeInfo has a vestigial 'length' field from when it used to be a
      FixedArray. This change removes that field, which saves some memory.
      
      More specifically:
      
      - Make ScopeInfo inherit from HeapObject, not FixedArrayBase which
        supplied the 'length' field.
      - Privatize the FixedArray-style functions that provide access to
        ScopeInfo fields by index, and move them from scope-info-inl.h to
        scope-info.cc. Those functions are still used pretty heavily during
        initialization (ScopeInfo::Create, etc.), but at least we can avoid
        presenting them to the rest of the world.
      - Change FactoryBase::NewScopeInfo to allocate the updated object shape.
        It maintains the existing behavior of filling the newly-allocated
        object with undefined, even though that's not a valid ScopeInfo and
        further initialization is required.
      - Move part of AccessorAssembler::ScriptContextTableLookup into a new
        Torque macro, because it used to rely on casting ScopeInfo to
        FixedArrayBase.
      - In V8HeapExplorer::AddEntry, don't claim that ScopeInfo objects are
        arrays. I think it makes more sense to list them under "(system)" in
        the dev tools, like most other V8 internal types.
      
      Bug: v8:8952
      Change-Id: I8278e3a90027d4409f0d268da0fe7080754c6b8c
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2601880Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
      Reviewed-by: 's avatarPeter Marshall <petermarshall@chromium.org>
      Reviewed-by: 's avatarDominik Inführ <dinfuehr@chromium.org>
      Reviewed-by: 's avatarNico Hartmann <nicohartmann@chromium.org>
      Reviewed-by: 's avatarMythri Alle <mythria@chromium.org>
      Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
      Cr-Commit-Position: refs/heads/master@{#72830}
      f731e13f
  4. 20 Jan, 2021 1 commit
    • Seth Brenith's avatar
      [torque] Begin porting ScopeInfo to Torque · ecaac329
      Seth Brenith authored
      This change adds Torque field definitions for ScopeInfo and begins to
      use the Torque-generated accessors in some places. It does not change
      the in-memory layout of ScopeInfo.
      
      Torque compiler changes:
      
      - Fix an issue where the parser created constexpr types for classes
        based on the class name rather than the `generates` clause. This meant
        that generated accessors referred to the imaginary type HashTable
        rather than the real C++ type FixedArray.
      - Don't pass Isolate* through the generated runtime functions that
        implement Torque macros. Maybe we'll need it eventually, but we don't
        right now and it complicates a lot of things.
      - Don't emit `kSomeFieldOffset` if some_field has an unknown offset.
        Instead, emit a member function `SomeFieldOffset()` which fetches the
        slice for some_field and returns its offset.
      - Emit an `AllocatedSize()` member function for classes which have
        complex length expressions. It fetches the slice for the last field
        and performs the multiply&add to compute the total object size.
      - Emit field accessors for fields with complex length expressions, using
        the new offset functions.
      - Fix a few minor bugs where Torque can write uncompilable code.
      
      With this change, most code still treats ScopeInfo like a FixedArray, so
      I would like to follow up with some additional changes:
      
      1. Generate a GC visitor for ScopeInfo and use it
      2. Generate accessors for struct-typed fields (indexed or otherwise),
         and use them
      3. Get rid of the FixedArray-style get and set accessors; use
         TaggedField::load and similar instead
      4. Inherit from HeapObject rather than FixedArrayBase to remove the
         unnecessary `length` field
      
      After that, there will only be one ugly part left: initialization. I
      think it's possible to generate a factory function that takes a bunch of
      iterator parameters and returns a fully-formed, verifiably correct
      ScopeInfo instance, but doing so is more complicated than the four
      mostly-mechanical changes listed above.
      
      Bug: v8:7793
      Change-Id: I55fcfe9189e4d1613c68d49e378da5dc02597b36
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2357758Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
      Reviewed-by: 's avatarPeter Marshall <petermarshall@chromium.org>
      Reviewed-by: 's avatarDominik Inführ <dinfuehr@chromium.org>
      Reviewed-by: 's avatarTobias Tebbi <tebbi@chromium.org>
      Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
      Cr-Commit-Position: refs/heads/master@{#72187}
      ecaac329