Changeset 245050 in webkit


Ignore:
Timestamp:
May 7, 2019 9:35:36 PM (5 years ago)
Author:
ysuzuki@apple.com
Message:

[JSC] LLIntPrototypeLoadAdaptiveStructureWatchpoint does not require Bag<>
​https://bugs.webkit.org/show_bug.cgi?id=197645

Reviewed by Saam Barati.

Source/JavaScriptCore:

We are using HashMap<std::tuple<Structure*, const Instruction*>, Bag<LLIntPrototypeLoadAdaptiveStructureWatchpoint>> for LLIntPrototypeLoadAdaptiveStructureWatchpoint,
but this has several memory inefficiency.

  1. Structure* and Instruction* are too large. We can just use StructureID and bytecodeOffset (unsigned).
  2. While we are using Bag<>, we do not add a new LLIntPrototypeLoadAdaptiveStructureWatchpoint after constructing this Bag first. So we can use Vector<LLIntPrototypeLoadAdaptiveStructureWatchpoint> instead. We ensure that new entry won't be added to this Vector by making Watchpoint non-movable.
  3. Instead of having OpGetById::Metadata&, we just hold unsigned bytecodeOffset, and get Metadata& from the owner CodeBlock when needed.
  • bytecode/CodeBlock.cpp:

(JSC::CodeBlock::finalizeLLIntInlineCaches):

  • bytecode/CodeBlock.h:
  • bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.cpp:

(JSC::LLIntPrototypeLoadAdaptiveStructureWatchpoint::LLIntPrototypeLoadAdaptiveStructureWatchpoint):
(JSC::LLIntPrototypeLoadAdaptiveStructureWatchpoint::fireInternal):

  • bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.h:
  • bytecode/Watchpoint.h:
  • llint/LLIntSlowPaths.cpp:

(JSC::LLInt::setupGetByIdPrototypeCache):

Source/WTF:

  • WTF.xcodeproj/project.pbxproj:
  • wtf/CMakeLists.txt:
  • wtf/Nonmovable.h: Copied from Source/JavaScriptCore/bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.h.
  • wtf/Vector.h:

(WTF::minCapacity>::uncheckedConstructAndAppend):

Location:
trunk/Source
Files:
11 edited
1 copied

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r245047 r245050  
     12019-05-07  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        [JSC] LLIntPrototypeLoadAdaptiveStructureWatchpoint does not require Bag<>
     4        https://bugs.webkit.org/show_bug.cgi?id=197645
     5
     6        Reviewed by Saam Barati.
     7
     8        We are using HashMap<std::tuple<Structure*, const Instruction*>, Bag<LLIntPrototypeLoadAdaptiveStructureWatchpoint>> for LLIntPrototypeLoadAdaptiveStructureWatchpoint,
     9        but this has several memory inefficiency.
     10
     11        1. Structure* and Instruction* are too large. We can just use StructureID and bytecodeOffset (unsigned).
     12        2. While we are using Bag<>, we do not add a new LLIntPrototypeLoadAdaptiveStructureWatchpoint after constructing this Bag first. So we can
     13           use Vector<LLIntPrototypeLoadAdaptiveStructureWatchpoint> instead. We ensure that new entry won't be added to this Vector by making Watchpoint
     14           non-movable.
     15        3. Instead of having OpGetById::Metadata&, we just hold `unsigned` bytecodeOffset, and get Metadata& from the owner CodeBlock when needed.
     16
     17        * bytecode/CodeBlock.cpp:
     18        (JSC::CodeBlock::finalizeLLIntInlineCaches):
     19        * bytecode/CodeBlock.h:
     20        * bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.cpp:
     21        (JSC::LLIntPrototypeLoadAdaptiveStructureWatchpoint::LLIntPrototypeLoadAdaptiveStructureWatchpoint):
     22        (JSC::LLIntPrototypeLoadAdaptiveStructureWatchpoint::fireInternal):
     23        * bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.h:
     24        * bytecode/Watchpoint.h:
     25        * llint/LLIntSlowPaths.cpp:
     26        (JSC::LLInt::setupGetByIdPrototypeCache):
     27
    1282019-05-07  Yusuke Suzuki  <ysuzuki@apple.com>
    229

  • trunk/Source/JavaScriptCore/bytecode/CodeBlock.cpp

    r245040 r245050  
    13031303    m_llintGetByIdWatchpointMap.removeIf([&] (const StructureWatchpointMap::KeyValuePairType& pair) -> bool {
    13041304        auto clear = [&] () {
    1305             const Instruction* instruction = std::get<1>(pair.key);
     1305            auto& instruction = instructions().at(std::get<1>(pair.key));
    13061306            OpcodeID opcode = instruction->opcodeID();
    13071307            if (opcode == op_get_by_id) {
    … …  
    13131313        };
    13141314
    1315         if (!vm.heap.isMarked(std::get<0>(pair.key)))
     1315        if (!vm.heap.isMarked(vm.heap.structureIDTable().get(std::get<0>(pair.key))))
    13161316            return clear();
    13171317
    1318         for (const LLIntPrototypeLoadAdaptiveStructureWatchpoint* watchpoint : pair.value) {
    1319             if (!watchpoint->key().isStillLive(vm))
     1318        for (const LLIntPrototypeLoadAdaptiveStructureWatchpoint& watchpoint : pair.value) {
     1319            if (!watchpoint.key().isStillLive(vm))
    13201320                return clear();
    13211321        }

  • trunk/Source/JavaScriptCore/bytecode/CodeBlock.h

    r245040 r245050  
    638638    }
    639639
    640     typedef HashMap<std::tuple<Structure*, const Instruction*>, Bag<LLIntPrototypeLoadAdaptiveStructureWatchpoint>> StructureWatchpointMap;
     640    typedef HashMap<std::tuple<StructureID, unsigned>, Vector<LLIntPrototypeLoadAdaptiveStructureWatchpoint>> StructureWatchpointMap;
    641641    StructureWatchpointMap& llintGetByIdWatchpointMap() { return m_llintGetByIdWatchpointMap; }
    642642

  • trunk/Source/JavaScriptCore/bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.cpp

    r243560 r245050  
    3333namespace JSC {
    3434
    35 LLIntPrototypeLoadAdaptiveStructureWatchpoint::LLIntPrototypeLoadAdaptiveStructureWatchpoint(CodeBlock* owner, const ObjectPropertyCondition& key, OpGetById::Metadata& getByIdMetadata)
     35LLIntPrototypeLoadAdaptiveStructureWatchpoint::LLIntPrototypeLoadAdaptiveStructureWatchpoint(CodeBlock* owner, const ObjectPropertyCondition& key, unsigned bytecodeOffset)
    3636    : m_owner(owner)
    3737    , m_key(key)
    38     , m_getByIdMetadata(getByIdMetadata)
     38    , m_bytecodeOffset(bytecodeOffset)
    3939{
    4040    RELEASE_ASSERT(key.watchingRequiresStructureTransitionWatchpoint());
    … …  
    5959    }
    6060
    61     clearLLIntGetByIdCache(m_getByIdMetadata);
     61    auto& instruction = m_owner->instructions().at(m_bytecodeOffset);
     62    clearLLIntGetByIdCache(instruction->as<OpGetById>().metadata(m_owner));
    6263}
    6364

  • trunk/Source/JavaScriptCore/bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.h

    r243560 r245050  
    3434class LLIntPrototypeLoadAdaptiveStructureWatchpoint final : public Watchpoint {
    3535public:
    36     LLIntPrototypeLoadAdaptiveStructureWatchpoint(CodeBlock*, const ObjectPropertyCondition&, OpGetById::Metadata&);
     36    LLIntPrototypeLoadAdaptiveStructureWatchpoint(CodeBlock*, const ObjectPropertyCondition&, unsigned bytecodeOffset);
    3737
    3838    void install(VM&);
    … …  
    4848    CodeBlock* m_owner;
    4949    ObjectPropertyCondition m_key;
    50     OpGetById::Metadata& m_getByIdMetadata;
     50    unsigned m_bytecodeOffset;
    5151};
    5252

  • trunk/Source/JavaScriptCore/bytecode/Watchpoint.h

    r239879 r245050  
    2929#include <wtf/FastMalloc.h>
    3030#include <wtf/Noncopyable.h>
     31#include <wtf/Nonmovable.h>
    3132#include <wtf/PrintStream.h>
    3233#include <wtf/ScopedLambda.h>
    … …  
    9293class Watchpoint : public BasicRawSentinelNode<Watchpoint> {
    9394    WTF_MAKE_NONCOPYABLE(Watchpoint);
     95    WTF_MAKE_NONMOVABLE(Watchpoint);
    9496    WTF_MAKE_FAST_ALLOCATED;
    9597public:

  • trunk/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp

    r244811 r245050  
    720720        return;
    721721
     722    unsigned bytecodeOffset = codeBlock->bytecodeOffset(pc);
    722723    PropertyOffset offset = invalidOffset;
    723724    CodeBlock::StructureWatchpointMap& watchpointMap = codeBlock->llintGetByIdWatchpointMap();
    724     Bag<LLIntPrototypeLoadAdaptiveStructureWatchpoint> watchpoints;
     725    Vector<LLIntPrototypeLoadAdaptiveStructureWatchpoint> watchpoints;
     726    watchpoints.reserveInitialCapacity(conditions.size());
    725727    for (ObjectPropertyCondition condition : conditions) {
    726728        if (!condition.isWatchable())
    … …  
    728730        if (condition.condition().kind() == PropertyCondition::Presence)
    729731            offset = condition.condition().offset();
    730         watchpoints.add(codeBlock, condition, metadata)->install(vm);
     732        watchpoints.uncheckedConstructAndAppend(codeBlock, condition, bytecodeOffset);
     733        watchpoints.last().install(vm);
    731734    }
    732735
    733736    ASSERT((offset == invalidOffset) == slot.isUnset());
    734     auto result = watchpointMap.add(std::make_tuple(structure, pc), WTFMove(watchpoints));
     737    auto result = watchpointMap.add(std::make_tuple(structure->id(), bytecodeOffset), WTFMove(watchpoints));
    735738    ASSERT_UNUSED(result, result.isNewEntry);
    736739

  • trunk/Source/WTF/ChangeLog

    r245039 r245050  
     12019-05-07  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        [JSC] LLIntPrototypeLoadAdaptiveStructureWatchpoint does not require Bag<>
     4        https://bugs.webkit.org/show_bug.cgi?id=197645
     5
     6        Reviewed by Saam Barati.
     7
     8        * WTF.xcodeproj/project.pbxproj:
     9        * wtf/CMakeLists.txt:
     10        * wtf/Nonmovable.h: Copied from Source/JavaScriptCore/bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.h.
     11        * wtf/Vector.h:
     12        (WTF::minCapacity>::uncheckedConstructAndAppend):
     13
    1142019-05-07  Eric Carlson  <eric.carlson@apple.com>
    215

  • trunk/Source/WTF/WTF.xcodeproj/project.pbxproj

    r244652 r245050  
    670670                E3CF76902115D6BA0091DE48 /* CompactPointerTuple.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CompactPointerTuple.h; sourceTree = "<group>"; };
    671671                E3E158251EADA53C004A079D /* SystemFree.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SystemFree.h; sourceTree = "<group>"; };
     672                E3E64F0B22813428001E55B4 /* Nonmovable.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = Nonmovable.h; sourceTree = "<group>"; };
    672673                E431CC4A21187ADB000C8A07 /* DispatchSPI.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DispatchSPI.h; sourceTree = "<group>"; };
    673674                E4A0AD371A96245500536DF6 /* WorkQueue.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WorkQueue.cpp; sourceTree = "<group>"; };
    … …  
    10401041                                0F0D85B317234CB100338210 /* NoLock.h */,
    10411042                                A8A472D0151A825B004123FF /* Noncopyable.h */,
     1043                                E3E64F0B22813428001E55B4 /* Nonmovable.h */,
    10421044                                526AEC911F6B4E5C00695F5D /* NoTailCalls.h */,
    10431045                                7CEAE5AC1EA6E10F00DB6890 /* NotFound.h */,

  • trunk/Source/WTF/wtf/CMakeLists.txt

    r244907 r245050  
    140140    NoTailCalls.h
    141141    Noncopyable.h
     142    Nonmovable.h
    142143    NotFound.h
    143144    NumberOfCores.h

  • trunk/Source/WTF/wtf/Nonmovable.h

    r245049 r245050  
    11/*
    2  * Copyright (C) 2016 Apple Inc. All rights reserved.
     2 * Copyright (C) 2019 Apple Inc. All Rights Reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    2626#pragma once
    2727
    28 #include "BytecodeStructs.h"
    29 #include "ObjectPropertyCondition.h"
    30 #include "Watchpoint.h"
     28#define WTF_MAKE_NONMOVABLE(ClassName) \
     29    private: \
     30        ClassName(ClassName&&) = delete; \
     31        ClassName& operator=(ClassName&&) = delete; \
    3132
    32 namespace JSC {
    33 
    34 class LLIntPrototypeLoadAdaptiveStructureWatchpoint final : public Watchpoint {
    35 public:
    36     LLIntPrototypeLoadAdaptiveStructureWatchpoint(CodeBlock*, const ObjectPropertyCondition&, OpGetById::Metadata&);
    37 
    38     void install(VM&);
    39 
    40     static void clearLLIntGetByIdCache(OpGetById::Metadata&);
    41 
    42     const ObjectPropertyCondition& key() const { return m_key; }
    43 
    44 protected:
    45     void fireInternal(VM&, const FireDetail&) override;
    46 
    47 private:
    48     CodeBlock* m_owner;
    49     ObjectPropertyCondition m_key;
    50     OpGetById::Metadata& m_getByIdMetadata;
    51 };
    52 
    53 } // namespace JSC

  • trunk/Source/WTF/wtf/Vector.h

    r238031 r245050  
    776776    void uncheckedAppend(ValueType&& value) { uncheckedAppend<ValueType>(std::forward<ValueType>(value)); }
    777777    template<typename U> void uncheckedAppend(U&&);
     778    template<typename... Args> void uncheckedConstructAndAppend(Args&&...);
    778779
    779780    template<typename U> void append(const U*, size_t);
    … …  
    13891390
    13901391    new (NotNull, end()) T(std::forward<U>(value));
     1392    ++m_size;
     1393}
     1394
     1395template<typename T, size_t inlineCapacity, typename OverflowHandler, size_t minCapacity>
     1396template<typename... Args>
     1397ALWAYS_INLINE void Vector<T, inlineCapacity, OverflowHandler, minCapacity>::uncheckedConstructAndAppend(Args&&... args)
     1398{
     1399    ASSERT(size() < capacity());
     1400
     1401    asanBufferSizeWillChangeTo(m_size + 1);
     1402
     1403    new (NotNull, end()) T(std::forward<Args>(args)...);
    13911404    ++m_size;
    13921405}
Note: See TracChangeset for help on using the changeset viewer.