| Summary: | Lower minimum table size of WTF::HashTable to reduce memory usage. | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Andreas Kling <kling> | ||||||
| Component: | Web Template Framework | Assignee: | Andreas Kling <kling> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | andersca, benjamin, dpranke, ggaren, koivisto, naginenis, psolanki, rakuco, simon.fraser, webkit-bug-importer, webkit.review.bot, xan.lopez | ||||||
| Priority: | P2 | Keywords: | InRadar, Performance | ||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Andreas Kling
2012-10-04 07:37:43 PDT
Created attachment 167099 [details]
Patch
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. Created attachment 167101 [details]
Bomb's away!
I approve this message! 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 Comment on attachment 167101 [details] Bomb's away! Clearing flags on attachment: 167101 Committed r130419: <http://trac.webkit.org/changeset/130419> All reviewed patches have been landed. Closing bug. 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. (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. I should say that it fails because "Infinity/Math.min(0,-0)" returns -Infinity instead of Infinity, (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. This was a dup of bug 86279 :( Reverted r130419 for reason: broke editing/pasteboard/data-transfer-items.html on chromium Committed r130436: <http://trac.webkit.org/changeset/130436> Comment on attachment 167101 [details] Bomb's away! Clearing flags on attachment: 167101 Committed r130643: <http://trac.webkit.org/changeset/130643> All reviewed patches have been landed. Closing bug. 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? (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. (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. 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. 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. |