Changeset 159395 in webkit


Ignore:
Timestamp:
Nov 17, 2013 6:10:42 PM (10 years ago)
Author:
fpizlo@apple.com
Message:

Simplify WatchpointSet state tracking
​https://bugs.webkit.org/show_bug.cgi?id=124465

Reviewed by Sam Weinig.

We previously represented the state of watchpoint sets using two booleans. But that
makes it awkward to case over the state.

We also previously supported a watchpoint set being both watched and invalidated. We
never used that capability, and its presence was just purely confusing.

This turns the whole thing into an enum.

  • assembler/MacroAssemblerARM64.h:

(JSC::MacroAssemblerARM64::branch8):

  • assembler/MacroAssemblerARMv7.h:

(JSC::MacroAssemblerARMv7::branch8):

  • assembler/MacroAssemblerX86.h:

(JSC::MacroAssemblerX86::branch8):

  • assembler/MacroAssemblerX86_64.h:

(JSC::MacroAssemblerX86_64::branch8):

  • bytecode/Watchpoint.cpp:

(JSC::WatchpointSet::WatchpointSet):
(JSC::WatchpointSet::add):
(JSC::WatchpointSet::notifyWriteSlow):
(JSC::InlineWatchpointSet::inflateSlow):

  • bytecode/Watchpoint.h:

(JSC::WatchpointSet::state):
(JSC::WatchpointSet::isStillValid):
(JSC::WatchpointSet::startWatching):
(JSC::WatchpointSet::notifyWrite):
(JSC::WatchpointSet::addressOfState):
(JSC::InlineWatchpointSet::InlineWatchpointSet):
(JSC::InlineWatchpointSet::hasBeenInvalidated):
(JSC::InlineWatchpointSet::startWatching):
(JSC::InlineWatchpointSet::notifyWrite):
(JSC::InlineWatchpointSet::decodeState):
(JSC::InlineWatchpointSet::encodeState):

  • jit/JITPropertyAccess.cpp:

(JSC::JIT::emitVarInjectionCheck):

  • jit/JITPropertyAccess32_64.cpp:

(JSC::JIT::emitVarInjectionCheck):

  • llint/LowLevelInterpreter.asm:
  • llint/LowLevelInterpreter32_64.asm:
  • llint/LowLevelInterpreter64.asm:
  • runtime/JSFunction.cpp:

(JSC::JSFunction::JSFunction):

  • runtime/JSFunctionInlines.h:

(JSC::JSFunction::JSFunction):

  • runtime/JSGlobalObject.cpp:

(JSC::JSGlobalObject::JSGlobalObject):

  • runtime/Structure.cpp:

(JSC::Structure::Structure):

  • runtime/SymbolTable.cpp:

(JSC::SymbolTableEntry::attemptToWatch):

  • runtime/SymbolTable.h:
Location:
trunk/Source/JavaScriptCore
Files:
18 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r159394 r159395  
     12013-11-16  Filip Pizlo  <fpizlo@apple.com>
     2
     3        Simplify WatchpointSet state tracking
     4        https://bugs.webkit.org/show_bug.cgi?id=124465
     5
     6        Reviewed by Sam Weinig.
     7       
     8        We previously represented the state of watchpoint sets using two booleans. But that
     9        makes it awkward to case over the state.
     10       
     11        We also previously supported a watchpoint set being both watched and invalidated. We
     12        never used that capability, and its presence was just purely confusing.
     13       
     14        This turns the whole thing into an enum.
     15
     16        * assembler/MacroAssemblerARM64.h:
     17        (JSC::MacroAssemblerARM64::branch8):
     18        * assembler/MacroAssemblerARMv7.h:
     19        (JSC::MacroAssemblerARMv7::branch8):
     20        * assembler/MacroAssemblerX86.h:
     21        (JSC::MacroAssemblerX86::branch8):
     22        * assembler/MacroAssemblerX86_64.h:
     23        (JSC::MacroAssemblerX86_64::branch8):
     24        * bytecode/Watchpoint.cpp:
     25        (JSC::WatchpointSet::WatchpointSet):
     26        (JSC::WatchpointSet::add):
     27        (JSC::WatchpointSet::notifyWriteSlow):
     28        (JSC::InlineWatchpointSet::inflateSlow):
     29        * bytecode/Watchpoint.h:
     30        (JSC::WatchpointSet::state):
     31        (JSC::WatchpointSet::isStillValid):
     32        (JSC::WatchpointSet::startWatching):
     33        (JSC::WatchpointSet::notifyWrite):
     34        (JSC::WatchpointSet::addressOfState):
     35        (JSC::InlineWatchpointSet::InlineWatchpointSet):
     36        (JSC::InlineWatchpointSet::hasBeenInvalidated):
     37        (JSC::InlineWatchpointSet::startWatching):
     38        (JSC::InlineWatchpointSet::notifyWrite):
     39        (JSC::InlineWatchpointSet::decodeState):
     40        (JSC::InlineWatchpointSet::encodeState):
     41        * jit/JITPropertyAccess.cpp:
     42        (JSC::JIT::emitVarInjectionCheck):
     43        * jit/JITPropertyAccess32_64.cpp:
     44        (JSC::JIT::emitVarInjectionCheck):
     45        * llint/LowLevelInterpreter.asm:
     46        * llint/LowLevelInterpreter32_64.asm:
     47        * llint/LowLevelInterpreter64.asm:
     48        * runtime/JSFunction.cpp:
     49        (JSC::JSFunction::JSFunction):
     50        * runtime/JSFunctionInlines.h:
     51        (JSC::JSFunction::JSFunction):
     52        * runtime/JSGlobalObject.cpp:
     53        (JSC::JSGlobalObject::JSGlobalObject):
     54        * runtime/Structure.cpp:
     55        (JSC::Structure::Structure):
     56        * runtime/SymbolTable.cpp:
     57        (JSC::SymbolTableEntry::attemptToWatch):
     58        * runtime/SymbolTable.h:
     59
    1602013-11-16  Filip Pizlo  <fpizlo@apple.com>
    261

  • trunk/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h

    r159261 r159395  
    16391639    }
    16401640   
     1641    Jump branch8(RelationalCondition cond, AbsoluteAddress left, TrustedImm32 right)
     1642    {
     1643        ASSERT(!(0xffffff00 & right.m_value));
     1644        load8(left, getCachedMemoryTempRegisterIDAndInvalidate());
     1645        return branch32(cond, memoryTempRegister, right);
     1646    }
     1647   
    16411648    Jump branchTest32(ResultCondition cond, RegisterID reg, RegisterID mask)
    16421649    {

  • trunk/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h

    r158208 r159395  
    13781378    }
    13791379   
     1380    Jump branch8(RelationalCondition cond, AbsoluteAddress left, TrustedImm32 right)
     1381    {
     1382        load8(left, addressTempRegister);
     1383        return branch32(cond, addressTempRegister, right);
     1384    }
     1385   
    13801386    Jump branchTest32(ResultCondition cond, RegisterID reg, RegisterID mask)
    13811387    {

  • trunk/Source/JavaScriptCore/assembler/MacroAssemblerX86.h

    r158057 r159395  
    5757    using MacroAssemblerX86Common::storeDouble;
    5858    using MacroAssemblerX86Common::convertInt32ToDouble;
     59    using MacroAssemblerX86Common::branch8;
    5960    using MacroAssemblerX86Common::branchTest8;
    6061
    … …  
    214215    }
    215216   
     217    Jump branch8(RelationalCondition cond, AbsoluteAddress left, TrustedImm32 right)
     218    {
     219        m_assembler.cmpb_im(right.m_value, left.m_ptr);
     220        return Jump(m_assembler.jCC(x86Condition(cond)));
     221    }
     222
    216223    Jump branchTest8(ResultCondition cond, AbsoluteAddress address, TrustedImm32 mask = TrustedImm32(-1))
    217224    {

  • trunk/Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h

    r158975 r159395  
    612612    }
    613613   
     614    using MacroAssemblerX86Common::branch8;
     615    Jump branch8(RelationalCondition cond, AbsoluteAddress left, TrustedImm32 right)
     616    {
     617        MacroAssemblerX86Common::move(TrustedImmPtr(left.m_ptr), scratchRegister);
     618        return MacroAssemblerX86Common::branch8(cond, Address(scratchRegister), right);
     619    }
     620   
    614621    using MacroAssemblerX86Common::branchTest8;
    615622    Jump branchTest8(ResultCondition cond, ExtendedAddress address, TrustedImm32 mask = TrustedImm32(-1))

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

    r158341 r159395  
    3939}
    4040
    41 WatchpointSet::WatchpointSet(InitialWatchpointSetMode mode)
    42     : m_isWatched(mode == InitializedWatching)
    43     , m_isInvalidated(false)
     41WatchpointSet::WatchpointSet(WatchpointState state)
     42    : m_state(state)
    4443{
    4544}
    … …  
    5857{
    5958    ASSERT(!isCompilationThread());
     59    ASSERT(state() != IsInvalidated);
    6060    if (!watchpoint)
    6161        return;
    6262    m_set.push(watchpoint);
    63     m_isWatched = true;
     63    m_state = IsWatched;
    6464}
    6565
    6666void WatchpointSet::notifyWriteSlow()
    6767{
    68     ASSERT(m_isWatched);
     68    ASSERT(state() == IsWatched);
    6969   
    7070    fireAllWatchpoints();
    71     m_isWatched = false;
    72     m_isInvalidated = true;
     71    m_state = IsInvalidated;
    7372    WTF::storeStoreFence();
    7473}
    … …  
    8988    ASSERT(isThin());
    9089    ASSERT(!isCompilationThread());
    91     WatchpointSet* fat = adoptRef(new WatchpointSet(InitializedBlind)).leakRef();
    92     if (m_data & IsInvalidatedFlag)
    93         fat->m_isInvalidated = true;
    94     if (m_data & IsWatchedFlag)
    95         fat->m_isWatched = true;
     90    WatchpointSet* fat = adoptRef(new WatchpointSet(decodeState(m_data))).leakRef();
    9691    WTF::storeStoreFence();
    9792    m_data = bitwise_cast<uintptr_t>(fat);

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

    r158341 r159395  
    4646};
    4747
    48 enum InitialWatchpointSetMode { InitializedWatching, InitializedBlind };
     48enum WatchpointState {
     49    ClearWatchpoint,
     50    IsWatched,
     51    IsInvalidated
     52};
    4953
    5054class InlineWatchpointSet;
    … …  
    5357    friend class LLIntOffsetsExtractor;
    5458public:
    55     WatchpointSet(InitialWatchpointSetMode);
     59    WatchpointSet(WatchpointState);
    5660    ~WatchpointSet(); // Note that this will not fire any of the watchpoints; if you need to know when a WatchpointSet dies then you need a separate mechanism for this.
     61   
     62    WatchpointState state() const { return static_cast<WatchpointState>(m_state); }
    5763   
    5864    // It is safe to call this from another thread.  It may return true
    … …  
    6571    {
    6672        WTF::loadLoadFence();
    67         return !m_isInvalidated;
     73        return state() != IsInvalidated;
    6874    }
    6975    // Like isStillValid(), may be called from another thread.
    … …  
    8086    // probably don't want to set watchpoints, since we typically don't want to
    8187    // set watchpoints that we believe will actually be fired.
    82     void startWatching() { m_isWatched = true; }
     88    void startWatching()
     89    {
     90        ASSERT(state() != IsInvalidated);
     91        m_state = IsWatched;
     92    }
    8393   
    8494    void notifyWrite()
    8595    {
    86         if (!m_isWatched)
     96        if (state() != IsWatched)
    8797            return;
    8898        notifyWriteSlow();
    8999    }
    90    
    91     bool* addressOfIsWatched() { return &m_isWatched; }
    92     bool* addressOfIsInvalidated() { return &m_isInvalidated; }
     100
     101    int8_t* addressOfState() { return &m_state; }
    93102   
    94103    JS_EXPORT_PRIVATE void notifyWriteSlow(); // Call only if you've checked isWatched.
    … …  
    100109   
    101110    SentinelLinkedList<Watchpoint, BasicRawSentinelNode<Watchpoint>> m_set;
    102     bool m_isWatched;
    103     bool m_isInvalidated;
     111    int8_t m_state;
    104112};
    105113
    … …  
    126134    WTF_MAKE_NONCOPYABLE(InlineWatchpointSet);
    127135public:
    128     InlineWatchpointSet(InitialWatchpointSetMode mode)
    129         : m_data((mode == InitializedWatching ? IsWatchedFlag : 0) | IsThinFlag)
     136    InlineWatchpointSet(WatchpointState state)
     137        : m_data(encodeState(state))
    130138    {
    131139    }
    … …  
    149157            return fat(data)->hasBeenInvalidated();
    150158        }
    151         return data & IsInvalidatedFlag;
     159        return decodeState(data) == IsInvalidated;
    152160    }
    153161   
    … …  
    166174            return;
    167175        }
    168         m_data |= IsWatchedFlag;
     176        ASSERT(decodeState(m_data) != IsInvalidated);
     177        m_data = encodeState(IsWatched);
    169178    }
    170179   
    … …  
    175184            return;
    176185        }
    177         if (!(m_data & IsWatchedFlag))
    178             return;
    179         m_data |= IsInvalidatedFlag;
     186        if (decodeState(m_data) == ClearWatchpoint)
     187            return;
     188        m_data = encodeState(IsInvalidated);
    180189        WTF::storeStoreFence();
    181190    }
    … …  
    183192private:
    184193    static const uintptr_t IsThinFlag        = 1;
    185     static const uintptr_t IsInvalidatedFlag = 2;
    186     static const uintptr_t IsWatchedFlag     = 4;
     194    static const uintptr_t StateMask         = 6;
     195    static const uintptr_t StateShift        = 1;
    187196   
    188197    static bool isThin(uintptr_t data) { return data & IsThinFlag; }
    189198    static bool isFat(uintptr_t data) { return !isThin(data); }
     199   
     200    static WatchpointState decodeState(uintptr_t data)
     201    {
     202        ASSERT(isThin(data));
     203        return static_cast<WatchpointState>((data & StateMask) >> StateShift);
     204    }
     205   
     206    static uintptr_t encodeState(WatchpointState state)
     207    {
     208        return (state << StateShift) | IsThinFlag;
     209    }
    190210   
    191211    bool isThin() const { return isThin(m_data); }

  • trunk/Source/JavaScriptCore/jit/JITPropertyAccess.cpp

    r159091 r159395  
    638638    if (!needsVarInjectionChecks)
    639639        return;
    640     addSlowCase(branchTest8(NonZero, AbsoluteAddress(m_codeBlock->globalObject()->varInjectionWatchpoint()->addressOfIsInvalidated())));
     640    addSlowCase(branch8(Equal, AbsoluteAddress(m_codeBlock->globalObject()->varInjectionWatchpoint()->addressOfState()), TrustedImm32(IsInvalidated)));
    641641}
    642642

  • trunk/Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp

    r159091 r159395  
    668668    if (!needsVarInjectionChecks)
    669669        return;
    670     addSlowCase(branchTest8(NonZero, AbsoluteAddress(m_codeBlock->globalObject()->varInjectionWatchpoint()->addressOfIsInvalidated())));
     670    addSlowCase(branch8(Equal, AbsoluteAddress(m_codeBlock->globalObject()->varInjectionWatchpoint()->addressOfState()), TrustedImm32(IsInvalidated)));
    671671}
    672672

  • trunk/Source/JavaScriptCore/llint/LowLevelInterpreter.asm

    r159276 r159395  
    7171const LowestTag = DeletedValueTag
    7272end
     73
     74# Watchpoint states
     75const ClearWatchpoint = 0
     76const IsWatched = 1
     77const IsInvalidated = 2
    7378
    7479# Some register conventions.

  • trunk/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm

    r159346 r159395  
    19881988    loadp CodeBlock::m_globalObject[t0], t0
    19891989    loadp JSGlobalObject::m_varInjectionWatchpoint[t0], t0
    1990     btbnz WatchpointSet::m_isInvalidated[t0], slowPath
     1990    bbeq WatchpointSet::m_state[t0], IsInvalidated, slowPath
    19911991end
    19921992

  • trunk/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm

    r159346 r159395  
    18011801    loadp CodeBlock::m_globalObject[t0], t0
    18021802    loadp JSGlobalObject::m_varInjectionWatchpoint[t0], t0
    1803     btbnz WatchpointSet::m_isInvalidated[t0], slowPath
     1803    bbeq WatchpointSet::m_state[t0], IsInvalidated, slowPath
    18041804end
    18051805

  • trunk/Source/JavaScriptCore/runtime/JSFunction.cpp

    r157330 r159395  
    9797    // clobbered once, and if it's clobbered more than once, that will probably only occur
    9898    // before we started optimizing, anyway.
    99     , m_allocationProfileWatchpoint(InitializedBlind)
     99    , m_allocationProfileWatchpoint(ClearWatchpoint)
    100100{
    101101}

  • trunk/Source/JavaScriptCore/runtime/JSFunctionInlines.h

    r153188 r159395  
    3636    , m_executable(vm, this, executable)
    3737    , m_scope(vm, this, scope)
    38     , m_allocationProfileWatchpoint(InitializedBlind) // See comment in JSFunction.cpp concerning the reason for using InitializedBlind as opposed to InitializedWatching.
     38    , m_allocationProfileWatchpoint(ClearWatchpoint) // See comment in JSFunction.cpp concerning the reason for using ClearWatchpoint as opposed to IsWatched.
    3939{
    4040}

  • trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp

    r159031 r159395  
    151151JSGlobalObject::JSGlobalObject(VM& vm, Structure* structure, const GlobalObjectMethodTable* globalObjectMethodTable)
    152152    : Base(vm, structure, 0)
    153     , m_masqueradesAsUndefinedWatchpoint(adoptRef(new WatchpointSet(InitializedWatching)))
    154     , m_havingABadTimeWatchpoint(adoptRef(new WatchpointSet(InitializedWatching)))
    155     , m_varInjectionWatchpoint(adoptRef(new WatchpointSet(InitializedWatching)))
     153    , m_masqueradesAsUndefinedWatchpoint(adoptRef(new WatchpointSet(IsWatched)))
     154    , m_havingABadTimeWatchpoint(adoptRef(new WatchpointSet(IsWatched)))
     155    , m_varInjectionWatchpoint(adoptRef(new WatchpointSet(IsWatched)))
    156156    , m_weakRandom(Options::forceWeakRandomSeed() ? Options::forcedWeakRandomSeed() : static_cast<unsigned>(randomNumber() * (std::numeric_limits<unsigned>::max() + 1.0)))
    157157    , m_evalEnabled(true)

  • trunk/Source/JavaScriptCore/runtime/Structure.cpp

    r157539 r159395  
    159159    , m_prototype(vm, this, prototype)
    160160    , m_classInfo(classInfo)
    161     , m_transitionWatchpointSet(InitializedWatching)
     161    , m_transitionWatchpointSet(IsWatched)
    162162    , m_offset(invalidOffset)
    163163    , m_typeInfo(typeInfo)
    … …  
    186186    , m_prototype(vm, this, jsNull())
    187187    , m_classInfo(info())
    188     , m_transitionWatchpointSet(InitializedWatching)
     188    , m_transitionWatchpointSet(IsWatched)
    189189    , m_offset(invalidOffset)
    190190    , m_typeInfo(CompoundType, OverridesVisitChildren)
    … …  
    208208    , m_prototype(vm, this, previous->storedPrototype())
    209209    , m_classInfo(previous->m_classInfo)
    210     , m_transitionWatchpointSet(InitializedWatching)
     210    , m_transitionWatchpointSet(IsWatched)
    211211    , m_offset(invalidOffset)
    212212    , m_typeInfo(previous->typeInfo().type(), previous->typeInfo().flags() & ~StructureHasRareData)

  • trunk/Source/JavaScriptCore/runtime/SymbolTable.cpp

    r159116 r159395  
    7373    FatEntry* entry = inflate();
    7474    if (!entry->m_watchpoints)
    75         entry->m_watchpoints = adoptRef(new WatchpointSet(InitializedWatching));
    76 }
    77 
    78 bool* SymbolTableEntry::addressOfIsWatched()
    79 {
    80     ASSERT(couldBeWatched());
    81     return fatEntry()->m_watchpoints->addressOfIsWatched();
     75        entry->m_watchpoints = adoptRef(new WatchpointSet(IsWatched));
    8276}
    8377

  • trunk/Source/JavaScriptCore/runtime/SymbolTable.h

    r159394 r159395  
    231231    void attemptToWatch();
    232232   
    233     bool* addressOfIsWatched();
    234    
    235233    void addWatchpoint(Watchpoint*);
    236234   
Note: See TracChangeset for help on using the changeset viewer.