- Revision
- 119341
- Author
- [email protected]
- Date
- 2012-06-02 15:49:05 -0700 (Sat, 02 Jun 2012)
Log Message
DOM string cache should hash pointers, not characters
https://bugs.webkit.org/show_bug.cgi?id=88175
Reviewed by Phil Pizlo and Sam Weinig.
../_javascript_Core:
* heap/Weak.h:
(JSC::weakAdd):
(JSC::weakRemove): Made these function templates slightly more generic
to accommodate new client types.
../WebCore:
Dromaeo DOM Core reports no change.
http://trac.webkit.org/changeset/84934 accidentally changed from hashing
pointers to hashing characters, due to template defaults. Let's change back.
Hashing characters is not so good because:
(1) It's not memory-safe with HashMap::set(). HashMap::set() replaces
the value but not the key. Since our values own our keys, we need to
ensure object identity between key and value, or the key can be freed
prematurely. (This is impossible to demonstrate with our current
eager sweep behavior, but it shows up as crashes in layout tests if you
change to lazy sweep.)
(2) It's slower.
* bindings/js/DOMWrapperWorld.h:
(WebCore): Override the default hash, which hashes based on characters.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (119340 => 119341)
--- trunk/Source/_javascript_Core/ChangeLog 2012-06-02 21:52:29 UTC (rev 119340)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-06-02 22:49:05 UTC (rev 119341)
@@ -1,3 +1,15 @@
+2012-06-02 Geoffrey Garen <[email protected]>
+
+ DOM string cache should hash pointers, not characters
+ https://bugs.webkit.org/show_bug.cgi?id=88175
+
+ Reviewed by Phil Pizlo and Sam Weinig.
+
+ * heap/Weak.h:
+ (JSC::weakAdd):
+ (JSC::weakRemove): Made these function templates slightly more generic
+ to accommodate new client types.
+
2012-06-01 Filip Pizlo <[email protected]>
DFG CFA should know that PutByVal can clobber the world
Modified: trunk/Source/_javascript_Core/heap/Weak.h (119340 => 119341)
--- trunk/Source/_javascript_Core/heap/Weak.h 2012-06-02 21:52:29 UTC (rev 119340)
+++ trunk/Source/_javascript_Core/heap/Weak.h 2012-06-02 22:49:05 UTC (rev 119341)
@@ -153,15 +153,15 @@
// This function helps avoid modifying a weak table while holding an iterator into it. (Object allocation
// can run a finalizer that modifies the table. We avoid that by requiring a pre-constructed object as our value.)
-template<typename T, typename U> inline void weakAdd(HashMap<T, Weak<U> >& map, const T& key, PassWeak<U> value)
+template<typename Map, typename Key, typename Value> inline void weakAdd(Map& map, const Key& key, Value value)
{
ASSERT(!map.get(key));
map.set(key, value); // The table may still have a zombie for value.
}
-template<typename T, typename U> inline void weakRemove(HashMap<T, Weak<U> >& map, const T& key, typename Weak<U>::GetType value)
+template<typename Map, typename Key, typename Value> inline void weakRemove(Map& map, const Key& key, Value value)
{
- typename HashMap<T, Weak<U> >::iterator it = map.find(key);
+ typename Map::iterator it = map.find(key);
ASSERT_UNUSED(value, value);
ASSERT(it != map.end());
ASSERT(it->second.was(value));
Modified: trunk/Source/WebCore/ChangeLog (119340 => 119341)
--- trunk/Source/WebCore/ChangeLog 2012-06-02 21:52:29 UTC (rev 119340)
+++ trunk/Source/WebCore/ChangeLog 2012-06-02 22:49:05 UTC (rev 119341)
@@ -1,3 +1,29 @@
+2012-06-02 Geoffrey Garen <[email protected]>
+
+ DOM string cache should hash pointers, not characters
+ https://bugs.webkit.org/show_bug.cgi?id=88175
+
+ Reviewed by Phil Pizlo and Sam Weinig.
+
+ Dromaeo DOM Core reports no change.
+
+ http://trac.webkit.org/changeset/84934 accidentally changed from hashing
+ pointers to hashing characters, due to template defaults. Let's change back.
+
+ Hashing characters is not so good because:
+
+ (1) It's not memory-safe with HashMap::set(). HashMap::set() replaces
+ the value but not the key. Since our values own our keys, we need to
+ ensure object identity between key and value, or the key can be freed
+ prematurely. (This is impossible to demonstrate with our current
+ eager sweep behavior, but it shows up as crashes in layout tests if you
+ change to lazy sweep.)
+
+ (2) It's slower.
+
+ * bindings/js/DOMWrapperWorld.h:
+ (WebCore): Override the default hash, which hashes based on characters.
+
2012-06-02 Eli Fidler <[email protected]>
Don't crash if we ask for fonts that don't exist.
Modified: trunk/Source/WebCore/WebCore.exp.in (119340 => 119341)
--- trunk/Source/WebCore/WebCore.exp.in 2012-06-02 21:52:29 UTC (rev 119340)
+++ trunk/Source/WebCore/WebCore.exp.in 2012-06-02 22:49:05 UTC (rev 119341)
@@ -1875,7 +1875,7 @@
__NPN_UTF8FromIdentifier
__ZN7WebCore16ScriptController20windowScriptNPObjectEv
__ZN7WebCore16ScriptController29cleanupScriptObjectsForPluginEPv
-__ZN7WebCore16jsStringSlowCaseEPN3JSC9ExecStateERN3WTF7HashMapIPNS3_10StringImplENS0_4WeakINS0_8JSStringEEENS3_10StringHashENS3_10HashTraitsIS6_EENSB_IS9_EEEES6_
+__ZN7WebCore16jsStringSlowCaseEPN3JSC9ExecStateERN3WTF7HashMapIPNS3_10StringImplENS0_4WeakINS0_8JSStringEEENS3_7PtrHashIS6_EENS3_10HashTraitsIS6_EENSC_IS9_EEEES6_
__ZN7WebCore17HTMLPlugInElement11getNPObjectEv
__ZNK7WebCore14SecurityOrigin9canAccessEPKS0_
__ZNK7WebCore4KURL7hasPathEv
Modified: trunk/Source/WebCore/bindings/js/DOMWrapperWorld.h (119340 => 119341)
--- trunk/Source/WebCore/bindings/js/DOMWrapperWorld.h 2012-06-02 21:52:29 UTC (rev 119340)
+++ trunk/Source/WebCore/bindings/js/DOMWrapperWorld.h 2012-06-02 22:49:05 UTC (rev 119341)
@@ -34,7 +34,7 @@
class ScriptController;
typedef HashMap<void*, JSC::Weak<JSDOMWrapper> > DOMObjectWrapperMap;
-typedef HashMap<StringImpl*, JSC::Weak<JSC::JSString> > JSStringCache;
+typedef HashMap<StringImpl*, JSC::Weak<JSC::JSString>, PtrHash<StringImpl*> > JSStringCache;
class JSStringOwner : public JSC::WeakHandleOwner {
public:
Modified: trunk/Source/WebKit2/win/WebKit2.def (119340 => 119341)
--- trunk/Source/WebKit2/win/WebKit2.def 2012-06-02 21:52:29 UTC (rev 119340)
+++ trunk/Source/WebKit2/win/WebKit2.def 2012-06-02 22:49:05 UTC (rev 119341)
@@ -183,7 +183,6 @@
?instrumentingAgentsForPage@InspectorInstrumentation@WebCore@@CAPAVInstrumentingAgents@2@PAVPage@2@@Z
?isCSSExclusionsEnabled@RuntimeEnabledFeatures@WebCore@@0_NA
?isPreloaded@CachedResourceLoader@WebCore@@QBE_NABVString@WTF@@@Z
- ?jsStringSlowCase@WebCore@@YA?AVJSValue@JSC@@PAVExecState@3@AAV?$HashMap@PAVStringImpl@WTF@@V?$Weak@VJSString@JSC@@@JSC@@UStringHash@2@U?$HashTraits@PAVStringImpl@WTF@@@2@U?$HashTraits@V?$Weak@VJSString@JSC@@@JSC@@@2@@WTF@@PAVStringImpl@6@@Z
?lastChangeWasUserEdit@HTMLTextFormControlElement@WebCore@@QBE_NXZ
?markersFor@DocumentMarkerController@WebCore@@QAE?AV?$Vector@PAVDocumentMarker@WebCore@@$0A@@WTF@@PAVNode@2@VMarkerTypes@DocumentMarker@2@@Z
?nextSibling@ComposedShadowTreeWalker@WebCore@@QAEXXZ
Modified: trunk/Source/WebKit2/win/WebKit2CFLite.def (119340 => 119341)
--- trunk/Source/WebKit2/win/WebKit2CFLite.def 2012-06-02 21:52:29 UTC (rev 119340)
+++ trunk/Source/WebKit2/win/WebKit2CFLite.def 2012-06-02 22:49:05 UTC (rev 119341)
@@ -176,7 +176,6 @@
?instrumentingAgentsForPage@InspectorInstrumentation@WebCore@@CAPAVInstrumentingAgents@2@PAVPage@2@@Z
?isCSSExclusionsEnabled@RuntimeEnabledFeatures@WebCore@@0_NA
?isPreloaded@CachedResourceLoader@WebCore@@QBE_NABVString@WTF@@@Z
- ?jsStringSlowCase@WebCore@@YA?AVJSValue@JSC@@PAVExecState@3@AAV?$HashMap@PAVStringImpl@WTF@@V?$Weak@VJSString@JSC@@@JSC@@UStringHash@2@U?$HashTraits@PAVStringImpl@WTF@@@2@U?$HashTraits@V?$Weak@VJSString@JSC@@@JSC@@@2@@WTF@@PAVStringImpl@6@@Z
?lastChangeWasUserEdit@HTMLTextFormControlElement@WebCore@@QBE_NXZ
?markersFor@DocumentMarkerController@WebCore@@QAE?AV?$Vector@PAVDocumentMarker@WebCore@@$0A@@WTF@@PAVNode@2@VMarkerTypes@DocumentMarker@2@@Z
?nextSibling@ComposedShadowTreeWalker@WebCore@@QAEXXZ
Modified: trunk/Source/autotools/symbols.filter (119340 => 119341)
--- trunk/Source/autotools/symbols.filter 2012-06-02 21:52:29 UTC (rev 119340)
+++ trunk/Source/autotools/symbols.filter 2012-06-02 22:49:05 UTC (rev 119341)
@@ -57,7 +57,7 @@
_ZN7WebCore15setDOMExceptionEPN3JSC9ExecStateEi;
_ZN7WebCore16HTMLInputElement17setSuggestedValueERKN3WTF6StringE;
_ZN7WebCore16HTMLInputElement15setEditingValueERKN3WTF6StringE;
-_ZN7WebCore16jsStringSlowCaseEPN3JSC9ExecStateERN3WTF7HashMapIPNS3_10StringImplENS0_4WeakINS0_8JSStringEEENS3_10StringHashENS3_10HashTraitsIS6_EENSB_IS9_EEEES6_;
+_ZN7WebCore16jsStringSlowCaseEPN3JSC9ExecStateERN3WTF7HashMapIPNS3_10StringImplENS0_4WeakINS0_8JSStringEEENS3_7PtrHashIS6_EENS3_10HashTraitsIS6_EENSC_IS9_EEEES6_;
_ZN7WebCore16scriptNameToCodeERKN3WTF6StringE;
_ZN7WebCore17cacheDOMStructureEPNS_17JSDOMGlobalObjectEPN3JSC9StructureEPKNS2_9ClassInfoE;
_ZN7WebCore17InspectorCounters12counterValueENS0_11CounterTypeE;
