Changeset 29943 in webkit for trunk/JavaScriptCore


Ignore:
Timestamp:
Feb 2, 2008, 4:20:34 PM (17 years ago)
Author:
[email protected]
Message:

JavaScriptCore:

Reviewed by Geoff Garen.

PLT speedup related to <rdar://problem/5659272> REGRESSION: PLT .4%
slower due to r28884 (global variable symbol table optimization)

Geoff's theory is that the slowdown was due to copying hash tables when
putting things into the back/forward cache. If that's true, then this
should fix the problem.


(According to Geoff's measurements, in a PLT that exaggerates the
importance of symbol table saving during cached page creation, this
patch is a ~3X speedup in cached page creation, and a 9% speedup overall.)

  • kjs/JSVariableObject.cpp: (KJS::JSVariableObject::saveLocalStorage): Updated for changes to SavedProperty, which has been revised to avoid initializing each SavedProperty twice when building the array. Store the property names too, so we don't have to store the symbol table separately. Do this by iterating the symbol table instead of the local storage vector. (KJS::JSVariableObject::restoreLocalStorage): Ditto. Restore the symbol table as well as the local storage vector.
  • kjs/JSVariableObject.h: Removed save/restoreSymbolTable and do that work inside save/restoreLocalStorage instead. Made restoreLocalStorage a non-const member function that takes a const reference to a SavedProperties object.
  • kjs/LocalStorage.h: Changed attributes to be unsigned instead of int to match other declarations of attributes elsewhere.
  • kjs/property_map.cpp: (KJS::SavedProperties::SavedProperties): Updated for data member name change. (KJS::PropertyMap::save): Updated for data member name change and to use the new inline init function instead of setting the fields directly. This allows us to skip initializing the SavedProperty objects when first allocating the array, and just do it when we're actually setting up the individual elements. (KJS::PropertyMap::restore): Updated for SavedProperty changes.
  • kjs/property_map.h: Changed SavedProperty from a struct to a class. Set it up so it does not get initialized at construction time to avoid initializing twice when creating an array of SavedProperty. Removed the m_ prefixes from the members of the SavedProperties struct. Generally we use m_ for class members and not struct.

WebCore:

Reviewed by Geoff Garen.

PLT speedup related to <rdar://problem/5659272> REGRESSION: PLT .4%
slower due to r28884 (global variable symbol table optimization)

  • history/CachedPage.cpp: (WebCore::CachedPage::CachedPage): Removed saveSymbolTable call. (WebCore::CachedPage::restore): Removed restoreSymbolTable call. (WebCore::CachedPage::clear): Removed clear of m_windowSymbolTable.
  • history/CachedPage.h: Removed m_windowSymbolTable, since save/restoreLocalStorage now takes care of the symbol table. Also removed many unnecessary includes.
Location:
trunk/JavaScriptCore
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/JavaScriptCore/ChangeLog

    r29941 r29943  
     12008-02-02  Darin Adler  <[email protected]>
     2
     3        Reviewed by Geoff Garen.
     4
     5        PLT speedup related to <rdar://problem/5659272> REGRESSION: PLT .4%
     6        slower due to r28884 (global variable symbol table optimization)
     7
     8        Geoff's theory is that the slowdown was due to copying hash tables when
     9        putting things into the back/forward cache. If that's true, then this
     10        should fix the problem.
     11       
     12        (According to Geoff's measurements, in a PLT that exaggerates the
     13        importance of symbol table saving during cached page creation, this
     14        patch is a ~3X speedup in cached page creation, and a 9% speedup overall.)
     15
     16        * JavaScriptCore.exp: Updated.
     17
     18        * kjs/JSVariableObject.cpp:
     19        (KJS::JSVariableObject::saveLocalStorage): Updated for changes to SavedProperty,
     20        which has been revised to avoid initializing each SavedProperty twice when building
     21        the array. Store the property names too, so we don't have to store the symbol table
     22        separately. Do this by iterating the symbol table instead of the local storage vector.
     23        (KJS::JSVariableObject::restoreLocalStorage): Ditto. Restore the symbol table as
     24        well as the local storage vector.
     25
     26        * kjs/JSVariableObject.h: Removed save/restoreSymbolTable and do that work inside
     27        save/restoreLocalStorage instead. Made restoreLocalStorage a non-const member function
     28        that takes a const reference to a SavedProperties object.
     29
     30        * kjs/LocalStorage.h: Changed attributes to be unsigned instead of int to match
     31        other declarations of attributes elsewhere.
     32
     33        * kjs/property_map.cpp:
     34        (KJS::SavedProperties::SavedProperties): Updated for data member name change.
     35        (KJS::PropertyMap::save): Updated for data member name change and to use the new
     36        inline init function instead of setting the fields directly. This allows us to
     37        skip initializing the SavedProperty objects when first allocating the array, and
     38        just do it when we're actually setting up the individual elements.
     39        (KJS::PropertyMap::restore): Updated for SavedProperty changes.
     40
     41        * kjs/property_map.h: Changed SavedProperty from a struct to a class. Set it up so
     42        it does not get initialized at construction time to avoid initializing twice when
     43        creating an array of SavedProperty. Removed the m_ prefixes from the members of
     44        the SavedProperties struct. Generally we use m_ for class members and not struct.
     45
    1462008-02-02  Tony Chang  <[email protected]>
    247

  • trunk/JavaScriptCore/JavaScriptCore.exp

    r29810 r29943  
    157157__ZN3KJS16JSVariableObject14deletePropertyEPNS_9ExecStateERKNS_10IdentifierE
    158158__ZN3KJS16JSVariableObject16getPropertyNamesEPNS_9ExecStateERNS_17PropertyNameArrayE
     159__ZN3KJS16JSVariableObject19restoreLocalStorageERKNS_15SavedPropertiesE
    159160__ZN3KJS16ParserRefCounted3refEv
    160161__ZN3KJS16ParserRefCounted5derefEv
    … …  
    246247__ZNK3KJS13ArrayInstance7getItemEj
    247248__ZNK3KJS14JSGlobalObject12saveBuiltinsERNS_13SavedBuiltinsE
    248 __ZNK3KJS16JSVariableObject15saveSymbolTableERN3WTF7HashMapINS1_6RefPtrINS_7UString3RepEEEmNS_17IdentifierRepHashENS_23IdentifierRepHashTraitsENS_26SymbolTableIndexHashTraitsEEE
    249249__ZNK3KJS16JSVariableObject16saveLocalStorageERNS_15SavedPropertiesE
    250 __ZNK3KJS16JSVariableObject18restoreSymbolTableERN3WTF7HashMapINS1_6RefPtrINS_7UString3RepEEEmNS_17IdentifierRepHashENS_23IdentifierRepHashTraitsENS_26SymbolTableIndexHashTraitsEEE
    251 __ZNK3KJS16JSVariableObject19restoreLocalStorageERNS_15SavedPropertiesE
    252250__ZNK3KJS19InternalFunctionImp14implementsCallEv
    253251__ZNK3KJS19InternalFunctionImp21implementsHasInstanceEv

  • trunk/JavaScriptCore/kjs/JSVariableObject.cpp

    r29663 r29943  
    11/*
    2  * Copyright (C) 2007 Apple Inc. All rights reserved.
     2 * Copyright (C) 2007, 2008 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    3737UString::Rep* IdentifierRepHashTraits::nullRepPtr = &UString::Rep::null; // Didn't want to make a whole source file for just this.
    3838
    39 void JSVariableObject::saveSymbolTable(SymbolTable& s) const
    40 {
    41     s = *d->symbolTable;
    42 }
    43 
    44 void JSVariableObject::restoreSymbolTable(SymbolTable& s) const
    45 {
    46     *d->symbolTable = s;
    47 }
    48 
    4939void JSVariableObject::saveLocalStorage(SavedProperties& p) const
    5040{
    51     unsigned count = d->localStorage.size();
     41    ASSERT(d->symbolTable);
     42    ASSERT(static_cast<size_t>(d->symbolTable->size()) == d->localStorage.size());
    5243
    53     p.m_properties.clear();
    54     p.m_count = count;
     44    unsigned count = d->symbolTable->size();
     45
     46    p.properties.clear();
     47    p.count = count;
    5548
    5649    if (!count)
    5750        return;
    5851
    59     p.m_properties.set(new SavedProperty[count]);
    60    
    61     SavedProperty* prop = p.m_properties.get();
    62     for (size_t i = 0; i < count; ++i, ++prop) {
    63         LocalStorageEntry& entry = d->localStorage[i];
    64         prop->value = entry.value;
    65         prop->attributes = entry.attributes;
     52    p.properties.set(new SavedProperty[count]);
     53
     54    SymbolTable::const_iterator end = d->symbolTable->end();
     55    for (SymbolTable::const_iterator it = d->symbolTable->begin(); it != end; ++it) {
     56        size_t i = it->second;
     57        const LocalStorageEntry& entry = d->localStorage[i];
     58        p.properties[i].init(it->first.get(), entry.value, entry.attributes);
    6659    }
    6760}
    6861
    69 void JSVariableObject::restoreLocalStorage(SavedProperties& p) const
     62void JSVariableObject::restoreLocalStorage(const SavedProperties& p)
    7063{
    71     unsigned count = p.m_count;
     64    unsigned count = p.count;
     65    d->symbolTable->clear();
    7266    d->localStorage.resize(count);
    73     SavedProperty* prop = p.m_properties.get();
    74     for (size_t i = 0; i < count; ++i, ++prop)
    75         d->localStorage[i] = LocalStorageEntry(prop->value, prop->attributes);
     67    SavedProperty* property = p.properties.get();
     68    for (size_t i = 0; i < count; ++i, ++property) {
     69        ASSERT(!d->symbolTable->contains(property->name()));
     70        LocalStorageEntry& entry = d->localStorage[i];
     71        d->symbolTable->set(property->name(), i);
     72        entry.value = property->value();
     73        entry.attributes = property->attributes();
     74    }
    7675}
    7776

  • trunk/JavaScriptCore/kjs/JSVariableObject.h

    r29818 r29943  
    11/*
    2  * Copyright (C) 2007 Apple Inc. All rights reserved.
     2 * Copyright (C) 2007, 2008 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    4141        LocalStorage& localStorage() { return d->localStorage; }
    4242       
    43         void saveSymbolTable(SymbolTable& s) const;
    44         void restoreSymbolTable(SymbolTable& s) const;
    45 
    46         void saveLocalStorage(SavedProperties& s) const;
    47         void restoreLocalStorage(SavedProperties& s) const;
     43        void saveLocalStorage(SavedProperties&) const;
     44        void restoreLocalStorage(const SavedProperties&);
    4845       
    4946        virtual bool deleteProperty(ExecState*, const Identifier&);
    … …  
    5855        struct JSVariableObjectData {
    5956            JSVariableObjectData() { }
    60 
    6157            JSVariableObjectData(SymbolTable* s)
    6258                : symbolTable(s) // Subclass owns this pointer.
    … …  
    6662            LocalStorage localStorage; // Storage for variables in the symbol table.
    6763            SymbolTable* symbolTable; // Maps name -> index in localStorage.
    68 
    6964        };
    7065
    … …  
    107102            return true;
    108103        }
    109 
    110104        return false;
    111105    }

  • trunk/JavaScriptCore/kjs/LocalStorage.h

    r29663 r29943  
    22/*
    33 *  Copyright (C) 1999-2000 Harri Porten ([email protected])
    4  *  Copyright (C) 2003, 2006, 2007 Apple Inc. All rights reserved.
     4 *  Copyright (C) 2003, 2006, 2007, 2008 Apple Inc. All rights reserved.
    55 *  Copyright (C) 2007 Cameron Zwarich ([email protected])
    66 *  Copyright (C) 2007 Maks Orlovich
    … …  
    3737        }
    3838   
    39         LocalStorageEntry(JSValue* v, int a)
     39        LocalStorageEntry(JSValue* v, unsigned a)
    4040            : value(v)
    4141            , attributes(a)
    … …  
    4444
    4545        JSValue* value;
    46         int attributes;
     46        unsigned attributes;
    4747    };
    4848

  • trunk/JavaScriptCore/kjs/property_map.cpp

    r28884 r29943  
    11/*
    2  *  Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
     2 *  Copyright (C) 2004, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
    33 *
    44 *  This library is free software; you can redistribute it and/or
    … …  
    127127
    128128SavedProperties::SavedProperties()
    129     : m_count(0)
     129    : count(0)
    130130{
    131131}
    … …  
    717717}
    718718
    719 void PropertyMap::save(SavedProperties &p) const
     719void PropertyMap::save(SavedProperties& s) const
    720720{
    721721    unsigned count = 0;
    … …  
    733733    }
    734734
    735     p.m_properties.clear();
    736     p.m_count = count;
     735    s.properties.clear();
     736    s.count = count;
    737737
    738738    if (count == 0)
    739739        return;
    740740   
    741     p.m_properties.set(new SavedProperty [count]);
    742    
    743     SavedProperty* prop = p.m_properties.get();
    744    
    745     if (!m_usingTable) {
    746 #if USE_SINGLE_ENTRY
    747         if (m_singleEntryKey && !(m_singleEntryAttributes & (ReadOnly | Function))) {
    748             prop->key = Identifier(m_singleEntryKey);
    749             prop->value = m_u.singleEntryValue;
    750             prop->attributes = m_singleEntryAttributes;
    751             ++prop;
    752         }
    753 #endif
    754     } else {
    755         // Save in the right order so we don't lose the order.
    756         // Another possibility would be to save the indices.
    757 
    758         // Allocate a buffer to use to sort the keys.
    759         Vector<Entry*, smallMapThreshold> sortedEntries(count);
    760 
    761         // Get pointers to the entries in the buffer.
    762         Entry** p = sortedEntries.data();
    763         unsigned entryCount = m_u.table->keyCount + m_u.table->deletedSentinelCount;
    764         for (unsigned i = 1; i <= entryCount; ++i) {
    765             if (m_u.table->entries()[i].key && !(m_u.table->entries()[i].attributes & (ReadOnly | Function)))
    766                 *p++ = &m_u.table->entries()[i];
    767         }
    768         ASSERT(p == sortedEntries.data() + count);
    769 
    770         // Sort the entries by index.
    771         qsort(sortedEntries.data(), p - sortedEntries.data(), sizeof(Entry*), comparePropertyMapEntryIndices);
    772 
    773         // Put the sorted entries into the saved properties list.
    774         for (Entry** q = sortedEntries.data(); q != p; ++q, ++prop) {
    775             Entry* e = *q;
    776             prop->key = Identifier(e->key);
    777             prop->value = e->value;
    778             prop->attributes = e->attributes;
    779         }
    780     }
    781 }
    782 
    783 void PropertyMap::restore(const SavedProperties &p)
    784 {
    785     for (unsigned i = 0; i != p.m_count; ++i)
    786         put(p.m_properties[i].key, p.m_properties[i].value, p.m_properties[i].attributes);
     741    s.properties.set(new SavedProperty[count]);
     742   
     743    SavedProperty* prop = s.properties.get();
     744   
     745#if USE_SINGLE_ENTRY
     746    if (!m_usingTable) {
     747        prop->init(m_singleEntryKey, m_u.singleEntryValue, m_singleEntryAttributes);
     748        return;
     749    }
     750#endif
     751
     752    // Save in the right order so we don't lose the order.
     753    // Another possibility would be to save the indices.
     754
     755    // Allocate a buffer to use to sort the keys.
     756    Vector<Entry*, smallMapThreshold> sortedEntries(count);
     757
     758    // Get pointers to the entries in the buffer.
     759    Entry** p = sortedEntries.data();
     760    unsigned entryCount = m_u.table->keyCount + m_u.table->deletedSentinelCount;
     761    for (unsigned i = 1; i <= entryCount; ++i) {
     762        if (m_u.table->entries()[i].key && !(m_u.table->entries()[i].attributes & (ReadOnly | Function)))
     763            *p++ = &m_u.table->entries()[i];
     764    }
     765    ASSERT(p == sortedEntries.data() + count);
     766
     767    // Sort the entries by index.
     768    qsort(sortedEntries.data(), p - sortedEntries.data(), sizeof(Entry*), comparePropertyMapEntryIndices);
     769
     770    // Put the sorted entries into the saved properties list.
     771    for (Entry** q = sortedEntries.data(); q != p; ++q, ++prop) {
     772        Entry* e = *q;
     773        prop->init(e->key, e->value, e->attributes);
     774    }
     775}
     776
     777void PropertyMap::restore(const SavedProperties& p)
     778{
     779    for (unsigned i = 0; i != p.count; ++i)
     780        put(Identifier(p.properties[i].name()), p.properties[i].value(), p.properties[i].attributes());
    787781}
    788782

  • trunk/JavaScriptCore/kjs/property_map.h

    r28884 r29943  
    11// -*- mode: c++; c-basic-offset: 4 -*-
    22/*
    3  *  Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
     3 *  Copyright (C) 2004, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
    44 *
    55 *  This library is free software; you can redistribute it and/or
    … …  
    3535    struct PropertyMapEntry;
    3636    struct PropertyMapHashTable;
    37    
    38     struct SavedProperty {
    39         Identifier key;
    40         ProtectedPtr<JSValue> value;
    41         unsigned attributes;
     37
     38    class SavedProperty : Noncopyable {
     39    public:
     40        // Since we use this in arrays, we allocate it uninitialized
     41        // and then explicitly initialize. This means we can allocate
     42        // the array without initializing every saved property in the
     43        // array twice. To accomplish this, the class uses data members
     44        // with types that don't have constructors.
     45        SavedProperty();
     46        void init(UString::Rep* name, JSValue*, unsigned attributes);
     47        ~SavedProperty();
     48
     49        UString::Rep* name() const;
     50        JSValue* value() const;
     51        unsigned attributes() const;
     52
     53    private:
     54        UString::Rep* m_name;
     55        JSValue* m_value;
     56        unsigned m_attributes;
    4257    };
    4358
    … …  
    4661        ~SavedProperties();
    4762       
    48         unsigned m_count;
    49         OwnArrayPtr<SavedProperty> m_properties;
     63        unsigned count;
     64        OwnArrayPtr<SavedProperty> properties;
    5065    };
    5166
    … …  
    107122    }
    108123
     124    inline SavedProperty::SavedProperty()
     125#ifndef NDEBUG
     126        : m_name(0)
     127        , m_value(0)
     128        , m_attributes(0)
     129#endif
     130    {
     131    }
     132
     133    inline void SavedProperty::init(UString::Rep* name, JSValue* value, unsigned attributes)
     134    {
     135        ASSERT(name);
     136        ASSERT(value);
     137
     138        ASSERT(!m_name);
     139        ASSERT(!m_value);
     140        ASSERT(!m_attributes);
     141
     142        m_name = name;
     143        m_value = value;
     144        m_attributes = attributes;
     145        name->ref();
     146        gcProtect(value);
     147    }
     148
     149    inline SavedProperty::~SavedProperty()
     150    {
     151        ASSERT(m_name);
     152        ASSERT(m_value);
     153
     154        m_name->deref();
     155        gcUnprotect(m_value);
     156    }
     157
     158    inline UString::Rep* SavedProperty::name() const
     159    {
     160        ASSERT(m_name);
     161        ASSERT(m_value);
     162
     163        return m_name;
     164    }
     165
     166    inline JSValue* SavedProperty::value() const
     167    {
     168        ASSERT(m_name);
     169        ASSERT(m_value);
     170
     171        return m_value;
     172    }
     173
     174    inline unsigned SavedProperty::attributes() const
     175    {
     176        ASSERT(m_name);
     177        ASSERT(m_value);
     178
     179        return m_attributes;
     180    }
     181
    109182} // namespace
    110183
Note: See TracChangeset for help on using the changeset viewer.