Bug 98406 – Lower minimum table size of WTF::HashTable to reduce memory usage.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Andreas Kling]

WTF's hash tables currently start out with a table size of 64 entries. A lot of this space ends up wasted, and my testing (PLT, JS benchmarks) indicates that we can bring it down as low as 8 without taking a performance hit. The memory savings from this would be borderline epic.

Should there be a performance regression from this, individual hash tables can be tweaked to start out with a larger size.

[Radar WebKit Bug Importer]

<rdar://problem/12432140>

[Andreas Kling]

Created attachment 167099 [details]
Patch

[Anders Carlsson]

Comment on attachment 167099 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=167099&action=review

> Source/WTF/wtf/HashTraits.h:54
> +        // The starting table size. Can be overridden when we know beforehand that a hash table will have
> +        // at least N entries.

I think putting the line break after "that" will look better.

[Andreas Kling]

Created attachment 167101 [details]
Bomb's away!

[Geoffrey Garen]

I approve this message!

[WebKit Review Bot]

Comment on attachment 167101 [details]
Bomb's away!

Rejecting attachment 167101 [details] from commit-queue.

New failing tests:
editing/pasteboard/data-transfer-items.html
Full output: http://queues.webkit.org/results/14152612

[Andreas Kling]

Comment on attachment 167101 [details]
Bomb's away!

Clearing flags on attachment: 167101

Committed r130419: <http://trac.webkit.org/changeset/130419>

[Andreas Kling]

All reviewed patches have been landed.  Closing bug.

[Raphael Kubo da Costa (:rakuco)]

Any chances of this having caused a regression in the JSC tests?

http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Debug/builds/6763/steps/jscore-test/logs/stdio

The Apple, GTK+ and Qt bots are failing similarly as well after this commit.

[Andreas Kling]

(In reply to comment #9)
> Any chances of this having caused a regression in the JSC tests?
> 
> http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Debug/builds/6763/steps/jscore-test/logs/stdio
> 
> The Apple, GTK+ and Qt bots are failing similarly as well after this commit.

I haven't a clue why this goes wrong, debugging now..

Basically, the following JS will print FAIL:

Math.min(1.0000000001,1)
print( Infinity/Math.min(0,-0) == Infinity ? "FAIL" : "PASS" )

If the first call to Math.min() is removed, it works correctly.

[Andreas Kling]

I should say that it fails because "Infinity/Math.min(0,-0)" returns -Infinity instead of Infinity,

[Andreas Kling]

(In reply to comment #11)
> I should say that it fails because "Infinity/Math.min(0,-0)" returns -Infinity instead of Infinity,

Sigh. I should say that correctly.
Infinity/Math.min(0,-0) returns Infinity instead of -Infinity. Unless the preceding call to Math.min(1.0000000001,1) is removed, in which case it correctly returns -Infinity. Back to debugging.

[Simon Fraser (smfr)]

This was a dup of bug 86279 :(

[Dirk Pranke]

Reverted r130419 for reason:

broke editing/pasteboard/data-transfer-items.html on chromium

Committed r130436: <http://trac.webkit.org/changeset/130436>

[Dirk Pranke]

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=editing%2Fpasteboard%2Fdata-transfer-items.html

[Andreas Kling]

Comment on attachment 167101 [details]
Bomb's away!

Clearing flags on attachment: 167101

Committed r130643: <http://trac.webkit.org/changeset/130643>

[Andreas Kling]

All reviewed patches have been landed.  Closing bug.

[Xan Lopez]

After some painful debug build bisecting (thanks Joanie!), it seems this is causing a very easy to trigger ASSERT in GTK+ debug builds:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff47c8aa8 in WTF::HashTable<WTF::RefPtr<WebCore::PluginPackage>, WTF::RefPtr<WebCore::PluginPackage>, WTF::IdentityExtractor, WebCore::PluginPackageHash, WTF::HashTraits<WTF::RefPtr<WebCore::PluginPackage> >, WTF::HashTraits<WTF::RefPtr<WebCore::PluginPackage> > >::add<WTF::IdentityHashTranslator<WebCore::PluginPackageHash>, WTF::RefPtr<WebCore::PluginPackage>, WTF::RefPtr<WebCore::PluginPackage> > (this=0x1102a40, key=..., extra=...) at ../../Source/WTF/wtf/HashTable.h:894
894	            ASSERT(result.iterator != end());
Missing separate debuginfos, use: debuginfo-install google-talkplugin-3.9.1.0-1.x86_64
(gdb) bt
#0  0x00007ffff47c8aa8 in WTF::HashTable<WTF::RefPtr<WebCore::PluginPackage>, WTF::RefPtr<WebCore::PluginPackage>, WTF::IdentityExtractor, WebCore::PluginPackageHash, WTF::HashTraits<WTF::RefPtr<WebCore::PluginPackage> >, WTF::HashTraits<WTF::RefPtr<WebCore::PluginPackage> > >::add<WTF::IdentityHashTranslator<WebCore::PluginPackageHash>, WTF::RefPtr<WebCore::PluginPackage>, WTF::RefPtr<WebCore::PluginPackage> > (this=0x1102a40, key=..., extra=...) at ../../Source/WTF/wtf/HashTable.h:894
#1  0x00007ffff47c6379 in WTF::HashTable<WTF::RefPtr<WebCore::PluginPackage>, WTF::RefPtr<WebCore::PluginPackage>, WTF::IdentityExtractor, WebCore::PluginPackageHash, WTF::HashTraits<WTF::RefPtr<WebCore::PluginPackage> >, WTF::HashTraits<WTF::RefPtr<WebCore::PluginPackage> > >::add (this=0x1102a40, value=...) at ../../Source/WTF/wtf/HashTable.h:393
#2  0x00007ffff47c4153 in WTF::HashSet<WTF::RefPtr<WebCore::PluginPackage>, WebCore::PluginPackageHash, WTF::HashTraits<WTF::RefPtr<WebCore::PluginPackage> > >::add (this=0x1102a40, value=...) at ../../Source/WTF/wtf/HashSet.h:183
#3  0x00007ffff47c2833 in WebCore::PluginDatabase::add (this=0x1102a00, prpPackage=...) at ../../Source/WebCore/plugins/PluginDatabase.cpp:330
#4  0x00007ffff47c1a5c in WebCore::PluginDatabase::refresh (this=0x1102a00) at ../../Source/WebCore/plugins/PluginDatabase.cpp:146
#5  0x00007ffff47c15c3 in WebCore::PluginDatabase::installedPlugins (populate=true) at ../../Source/WebCore/plugins/PluginDatabase.cpp:75
#6  0x00007ffff3d0332e in PlatformStrategiesGtk::getPluginInfo (this=0x57bc30, page=0x5c2800, outPlugins=...) at ../../Source/WebKit/gtk/WebCoreSupport/PlatformStrategiesGtk.cpp:75
#7  0x00007ffff47cc74a in WebCore::PluginData::initPlugins (this=0x12d5060, page=0x5c2800) at ../../Source/WebCore/plugins/PluginData.cpp:77
#8  0x00007ffff47cc494 in WebCore::PluginData::PluginData (this=0x12d5060, page=0x5c2800) at ../../Source/WebCore/plugins/PluginData.cpp:36
#9  0x00007ffff4656c15 in WebCore::PluginData::create (page=0x5c2800) at ../../Source/WebCore/plugins/PluginData.h:74
#10 0x00007ffff4653704 in WebCore::Page::pluginData (this=0x5c2800) at ../../Source/WebCore/page/Page.cpp:463
#11 0x00007ffff47c0762 in WebCore::DOMPluginArray::pluginData (this=0x130f390) at ../../Source/WebCore/plugins/DOMPluginArray.cpp:97
#12 0x00007ffff47c0571 in WebCore::DOMPluginArray::canGetItemsForName (this=0x130f390, propertyName=...) at ../../Source/WebCore/plugins/DOMPluginArray.cpp:61
#13 0x00007ffff3e4f4c7 in WebCore::JSDOMPluginArray::canGetItemsForName (pluginArray=0x130f390, propertyName=...) at ../../Source/WebCore/bindings/js/JSDOMPluginArrayCustom.cpp:33
#14 0x00007ffff4d8959e in WebCore::JSDOMPluginArray::getOwnPropertySlot (cell=0x7fff9a5cf4e0, exec=0x7fffa0095250, propertyName=..., slot=...) at DerivedSources/WebCore/JSDOMPluginArray.cpp:154
#15 0x00007ffff7849ab3 in JSC::JSCell::fastGetOwnPropertySlot (this=0x7fff9a5cf4e0, exec=0x7fffa0095250, propertyName=..., slot=...) at ../../Source/JavaScriptCore/runtime/JSObject.h:1017
#16 0x00007ffff794d0ec in JSC::JSValue::get (this=0x7fffffffb830, exec=0x7fffa0095250, propertyName=..., slot=...) at ../../Source/JavaScriptCore/runtime/JSObject.h:1273
#17 0x00007ffff794d031 in JSC::JSValue::get (this=0x7fffffffb830, exec=0x7fffa0095250, propertyName=...) at ../../Source/JavaScriptCore/runtime/JSObject.h:1260
#18 0x00007ffff7a50121 in JSC::LLInt::getByVal (exec=0x7fffa0095250, baseValue=..., subscript=...) at ../../Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:1031
#19 0x00007ffff7a49e73 in JSC::LLInt::llint_slow_path_get_by_val (exec=0x7fffa0095250, pc=0xe5bf28) at ../../Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:1037
#20 0x00007ffff7a52bd6 in llint_op_get_by_val () from /home/xan/git/webkit/build/debug/.libs/libjavascriptcoregtk-3.0.so
#21 0x00007fffffffb950 in ?? ()
#22 0x00007fff99195440 in ?? ()
#23 0x0000000000b29c90 in ?? ()
#24 0x00007fff99195580 in ?? ()
#25 0x00007fffffffb960 in ?? ()
#26 0x00007fff98576580 in ?? ()
#27 0x00007ffff7fe8ae0 in ?? () from /home/xan/git/webkit/build/debug/.libs/libjavascriptcoregtk-3.0.so
#28 0x00007ffff7286be0 in ?? () from /home/xan/git/webkit/build/debug/.libs/libwebkitgtk-3.0.so
#29 0x00007fffffffb990 in ?? ()
#30 0x00007fffa3fff380 in ?? ()
#31 0x00000000005ec7b8 in ?? ()
#32 0x00007fffa0095088 in ?? ()
#33 0x0000000000000000 in ?? ()

Not sure what's going on. Does some other value somewhere need to be tweaked to be in sync with this change?

[Andreas Kling]

(In reply to comment #18)
> After some painful debug build bisecting (thanks Joanie!), it seems this is causing a very easy to trigger ASSERT in GTK+ debug builds:
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x00007ffff47c8aa8 in WTF::HashTable<WTF::RefPtr<WebCore::PluginPackage>, WTF::RefPtr<WebCore::PluginPackage>, WTF::IdentityExtractor, WebCore::PluginPackageHash, WTF::HashTraits<WTF::RefPtr<WebCore::PluginPackage> >, WTF::HashTraits<WTF::RefPtr<WebCore::PluginPackage> > >::add<WTF::IdentityHashTranslator<WebCore::PluginPackageHash>, WTF::RefPtr<WebCore::PluginPackage>, WTF::RefPtr<WebCore::PluginPackage> > (this=0x1102a40, key=..., extra=...) at ../../Source/WTF/wtf/HashTable.h:894
> 894                ASSERT(result.iterator != end());
> Missing separate debuginfos, use: debuginfo-install google-talkplugin-3.9.1.0-1.x86_64
> (gdb) bt
> #0  0x00007ffff47c8aa8 in WTF::HashTable<WTF::RefPtr<WebCore::PluginPackage>, WTF::RefPtr<WebCore::PluginPackage>, WTF::IdentityExtractor, WebCore::PluginPackageHash, WTF::HashTraits<WTF::RefPtr<WebCore::PluginPackage> >, WTF::HashTraits<WTF::RefPtr<WebCore::PluginPackage> > >::add<WTF::IdentityHashTranslator<WebCore::PluginPackageHash>, WTF::RefPtr<WebCore::PluginPackage>, WTF::RefPtr<WebCore::PluginPackage> > (this=0x1102a40, key=..., extra=...) at ../../Source/WTF/wtf/HashTable.h:894
> #1  0x00007ffff47c6379 in WTF::HashTable<WTF::RefPtr<WebCore::PluginPackage>, WTF::RefPtr<WebCore::PluginPackage>, WTF::IdentityExtractor, WebCore::PluginPackageHash, WTF::HashTraits<WTF::RefPtr<WebCore::PluginPackage> >, WTF::HashTraits<WTF::RefPtr<WebCore::PluginPackage> > >::add (this=0x1102a40, value=...) at ../../Source/WTF/wtf/HashTable.h:393
> #2  0x00007ffff47c4153 in WTF::HashSet<WTF::RefPtr<WebCore::PluginPackage>, WebCore::PluginPackageHash, WTF::HashTraits<WTF::RefPtr<WebCore::PluginPackage> > >::add (this=0x1102a40, value=...) at ../../Source/WTF/wtf/HashSet.h:183
> #3  0x00007ffff47c2833 in WebCore::PluginDatabase::add (this=0x1102a00, prpPackage=...) at ../../Source/WebCore/plugins/PluginDatabase.cpp:330

My guess is that this is caused by the rickety looking PluginPackageHash here: http://trac.webkit.org/browser/trunk/Source/WebCore/plugins/PluginPackage.h#L127

Notice that hash(x) will hash m_path and m_lastModified, while equal(x, y) will just compare m_description, meaning that two RefPtr<PluginPackage> that are "equal" according to PluginPackageHash will have different hashes. Badness will then happen if they end up in the same hash bucket, much like on bug 98627. I'm not sure what the correct fix is here, maybe someone with plug-in experience could weigh in.

[Anders Carlsson]

(In reply to comment #19)
> (In reply to comment #18)
> > After some painful debug build bisecting (thanks Joanie!), it seems this is causing a very easy to trigger ASSERT in GTK+ debug builds:
> > 
> > Program received signal SIGSEGV, Segmentation fault.
> > 0x00007ffff47c8aa8 in WTF::HashTable<WTF::RefPtr<WebCore::PluginPackage>, WTF::RefPtr<WebCore::PluginPackage>, WTF::IdentityExtractor, WebCore::PluginPackageHash, WTF::HashTraits<WTF::RefPtr<WebCore::PluginPackage> >, WTF::HashTraits<WTF::RefPtr<WebCore::PluginPackage> > >::add<WTF::IdentityHashTranslator<WebCore::PluginPackageHash>, WTF::RefPtr<WebCore::PluginPackage>, WTF::RefPtr<WebCore::PluginPackage> > (this=0x1102a40, key=..., extra=...) at ../../Source/WTF/wtf/HashTable.h:894
> > 894                ASSERT(result.iterator != end());
> > Missing separate debuginfos, use: debuginfo-install google-talkplugin-3.9.1.0-1.x86_64
> > (gdb) bt
> > #0  0x00007ffff47c8aa8 in WTF::HashTable<WTF::RefPtr<WebCore::PluginPackage>, WTF::RefPtr<WebCore::PluginPackage>, WTF::IdentityExtractor, WebCore::PluginPackageHash, WTF::HashTraits<WTF::RefPtr<WebCore::PluginPackage> >, WTF::HashTraits<WTF::RefPtr<WebCore::PluginPackage> > >::add<WTF::IdentityHashTranslator<WebCore::PluginPackageHash>, WTF::RefPtr<WebCore::PluginPackage>, WTF::RefPtr<WebCore::PluginPackage> > (this=0x1102a40, key=..., extra=...) at ../../Source/WTF/wtf/HashTable.h:894
> > #1  0x00007ffff47c6379 in WTF::HashTable<WTF::RefPtr<WebCore::PluginPackage>, WTF::RefPtr<WebCore::PluginPackage>, WTF::IdentityExtractor, WebCore::PluginPackageHash, WTF::HashTraits<WTF::RefPtr<WebCore::PluginPackage> >, WTF::HashTraits<WTF::RefPtr<WebCore::PluginPackage> > >::add (this=0x1102a40, value=...) at ../../Source/WTF/wtf/HashTable.h:393
> > #2  0x00007ffff47c4153 in WTF::HashSet<WTF::RefPtr<WebCore::PluginPackage>, WebCore::PluginPackageHash, WTF::HashTraits<WTF::RefPtr<WebCore::PluginPackage> > >::add (this=0x1102a40, value=...) at ../../Source/WTF/wtf/HashSet.h:183
> > #3  0x00007ffff47c2833 in WebCore::PluginDatabase::add (this=0x1102a00, prpPackage=...) at ../../Source/WebCore/plugins/PluginDatabase.cpp:330
> 
> My guess is that this is caused by the rickety looking PluginPackageHash here: http://trac.webkit.org/browser/trunk/Source/WebCore/plugins/PluginPackage.h#L127
> 
> Notice that hash(x) will hash m_path and m_lastModified, while equal(x, y) will just compare m_description, meaning that two RefPtr<PluginPackage> that are "equal" according to PluginPackageHash will have different hashes. Badness will then happen if they end up in the same hash bucket, much like on bug 98627. I'm not sure what the correct fix is here, maybe someone with plug-in experience could weigh in.

Given how broken that hash is, I think you should just hard code the size to 64 for this hash table and file a FIXME bug.

[Eric Seidel (no email)]

Comment on attachment 167099 [details]
Patch

Cleared Anders Carlsson's review+ from obsolete attachment 167099 [details] so that this bug does not appear in http://webkit.org/pending-commit.

[Pratik Solanki]

The patch has been checked in and this bug is fixed. Moving back to Resolved.

Xan, can you file a separate bug for the GTK assertion if it is still an issue? We should track that separately instead of reopening this bug.