223757 2021-03-25 12:09:48 -0700 Embiggen maximum HashTable size when not using ANGLE 2021-04-19 09:59:40 -0700 1 1 1 Unclassified WebKit Web Template Framework WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 don.olmstead don.olmstead achristensen benjamin cdumez cmarcelo ews-watchlist sam webkit-bug-importer ysuzuki oldest_to_newest 1743858 0 don.olmstead 2021-03-25 12:09:48 -0700 A HashMap of sh::ShaderVariable is over the limit introduced in https://trac.webkit.org/changeset/274603/webkit In file included from ../../Source/WebCore/platform/graphics/opengl/GraphicsContextGLOpenGLCommon.cpp:30: In file included from ../../Source/WebCore/platform/graphics/opengl/GraphicsContextGLOpenGL.h:30: In file included from ../../Source/WebCore/platform/graphics\GraphicsContextGL.h:32: In file included from ../../Source/WebCore/platform/graphics/Image.h:30: In file included from ../../Source/WebCore/platform/graphics/DecodingOptions.h:28: In file included from ../../Source/WebCore/platform/graphics\IntSize.h:29: In file included from WTF/Headers\wtf/JSONValues.h:35: In file included from WTF/Headers\wtf/HashMap.h:25: WTF/Headers\wtf/HashTable.h:671:9: error: static_assert failed due to requirement 'sizeof(WTF::String) + sizeof(WTF::KeyValuePair<WTF::String, sh::ShaderVariable>) < 250' "Your HashTable types are too big to efficiently move when rehashing. Consider using UniqueRef instead" static_assert(sizeof(Key) + sizeof(Value) < 250, "Your HashTable types are too big to efficiently move when rehashing. Consider using UniqueRef instead"); ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ .... ^ ../../Source/WebCore/platform/graphics/opengl/GraphicsContextGLOpenGLCommon.cpp:301:63: note: in instantiation of member function 'WTF::HashMap<WTF::String, sh::ShaderVariable, WTF::DefaultHash<WTF::String>, WTF::HashTraits<WTF::String>, WTF::HashTraits<sh::ShaderVariable> >::find' requested here const auto& fragmentSymbol = fragmentEntry.varyingMap.find(symbolName); 1743864 1 424267 don.olmstead 2021-03-25 12:30:00 -0700 Created attachment 424267 Patch 1743865 2 424267 achristensen 2021-03-25 12:30:58 -0700 Comment on attachment 424267 Patch Unfortunately, this is the most straightforward solution. 1743892 3 ews-feeder 2021-03-25 13:20:52 -0700 Committed r275053: <https://commits.webkit.org/r275053> All reviewed patches have been landed. Closing bug and clearing flags on attachment 424267. 1743893 4 webkit-bug-importer 2021-03-25 13:21:15 -0700 <rdar://problem/75850257> 1744671 5 sam 2021-03-28 18:58:03 -0700 Out of interest, what makes using a UniqueRef, as the assert suggests should be done here, not feasible? 1744742 6 achristensen 2021-03-29 09:55:55 -0700 The maps that are hitting this assertion are our use of ShaderSymbolMap. The value, sh::ShaderVariable, is too big. Since sh::ShaderVariable isn't fast allocated, we can't use UniqueRef or makeUnique as they stand. Our solutions were either 1. Make sh::ShaderVariable fast allocated which would require ANGLE to use WTF 2. Use makeUniqueRefWithoutFastMallocCheck 3. This, which doesn't change existing behavior at all. Given that ports are moving away from !USE(ANGLE), we decided it would be best to just keep the status quo until that happens. 1744752 7 sam 2021-03-29 10:08:46 -0700 (In reply to Alex Christensen from comment #6) > The maps that are hitting this assertion are our use of ShaderSymbolMap. > The value, sh::ShaderVariable, is too big. Since sh::ShaderVariable isn't > fast allocated, we can't use UniqueRef or makeUnique as they stand. Our > solutions were either > > 1. Make sh::ShaderVariable fast allocated which would require ANGLE to use > WTF > 2. Use makeUniqueRefWithoutFastMallocCheck > 3. This, which doesn't change existing behavior at all. > > Given that ports are moving away from !USE(ANGLE), we decided it would be > best to just keep the status quo until that happens. I don't understand why makeUniqueRefWithoutFastMallocCheck wouldn't be the right answer here. Having !USE(ANGLE) in the middle of HashTable seems like more invasive change. 1744755 8 achristensen 2021-03-29 10:13:20 -0700 True. makeUniqueRefWithoutFastMallocCheck would be better. 1744769 9 achristensen 2021-03-29 10:38:18 -0700 The best solution is to remove all the !USE(ANGLE) code, but this is just in the interim. 424267 2021-03-25 12:30:00 -0700 2021-03-25 13:20:53 -0700 Patch bug-223757.diff text/plain 2288 don.olmstead ZGlmZiAtLWdpdCBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nIGIvU291cmNlL1dURi9DaGFuZ2VMb2cK aW5kZXggZjI5NDBkMzE4NDdmLi4wOWZiOTM1M2M4MGYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYv Q2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjUgQEAKKzIw MjEtMDMtMjUgIERvbiBPbG1zdGVhZCAgPGRvbi5vbG1zdGVhZEBzb255LmNvbT4KKworICAgICAg ICBFbWJpZ2dlbiBtYXhpbXVtIEhhc2hUYWJsZSBzaXplIHdoZW4gbm90IHVzaW5nIEFOR0xFCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMjM3NTcKKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUaGlzIGlzIGEg cGFydGlhbCByZXZlcnQgb2YgcjI3NDYwMyB3aGljaCBjdXQgdGhlIG1heGltdW0gc2l6ZSBIYXNo VGFibGUgZnJvbSA0MDAgdG8gMjUwLgorICAgICAgICBXaGVuIG5vdCB1c2luZyBBTkdMRSBkaXJl Y3RseSBmb3IgYW4gT3BlbkdMIEVTIGltcGxlbWVudGF0aW9uIHRoZXJlJ3Mgc29tZSBjb2RlIGhh c2hpbmcgaXRzCisgICAgICAgIHNoOjpTaGFkZXJWYXJpYWJsZSBmcm9tIHRoZSBzaGFkZXIgY29t cGlsZXIgQU5HTEUgc2hpcHMgd2l0aC4gVGhpcyB0eXBlIGlzIHRvbyBiaWcgYW5kCisgICAgICAg IHRyaWdnZXJzIHRoZSBzdGF0aWNfYXNzZXJ0LgorCisgICAgICAgIFRoZSBldmVudHVhbCBnb2Fs IGlzIGZvciBBTkdMRSB0byBiZSB0aGUgT3BlbkdMIEVTIGltcGxlbWVudGF0aW9uIGZvciBXZWJL aXQgd2hlbiBlbmFibGluZworICAgICAgICBXZWJHTC4gT24gUGxheVN0YXRpb24gd2UgYWxyZWFk eSBoYXZlIGEgc3lzdGVtIE9wZW5HTCBFUyBhbmQgYW55IHVzZSBvZiBXZWJHTCBpcyBpbiBsZWdh Y3kKKyAgICAgICAgYXBwbGljYXRpb25zLiBUaGUgV2ViR0wgY29tcGxpYW5jZSBmZWF0dXJlcyBp biBBTkdMRSBhcmVuJ3QgbmVlZGVkIGluIHRoaXMgY2FzZSBzbyBpZGVhbGx5CisgICAgICAgIHdl IHdvdWxkbid0IGV2ZW4gY29tcGlsZSBvdXQgQU5HTEUuCisKKyAgICAgICAgVGhpcyBwYXRjaCBw dW50cyBvbiBmaXhpbmcgdGhlIGFjdHVhbCBwcm9ibGVtIGFuZCBqdXN0IHJlc29sdmVzIHRoaXMg YnVpbGQgaXNzdWUuCisKKyAgICAgICAgKiB3dGYvSGFzaFRhYmxlLmg6CisgICAgICAgIChXVEY6 OktleVRyYWl0cz46OmlubGluZUxvb2t1cCk6CisKIDIwMjEtMDMtMjUgIENocmlzIER1bWV6ICA8 Y2R1bWV6QGFwcGxlLmNvbT4KIAogICAgICAgICBEbyBub3QgZG8gcHJvY2VzcyBwcmUtd2FybWlu ZyB3aGVuIHRoZSBzeXN0ZW0gaXMgdW5kZXIgbWVtb3J5IHByZXNzdXJlCmRpZmYgLS1naXQgYS9T b3VyY2UvV1RGL3d0Zi9IYXNoVGFibGUuaCBiL1NvdXJjZS9XVEYvd3RmL0hhc2hUYWJsZS5oCmlu ZGV4IGE1ZTZiNjJlYWQ1OS4uNmQ0YjJjOGY3MmIyIDEwMDY0NAotLS0gYS9Tb3VyY2UvV1RGL3d0 Zi9IYXNoVGFibGUuaAorKysgYi9Tb3VyY2UvV1RGL3d0Zi9IYXNoVGFibGUuaApAQCAtNjY4LDgg KzY2OCwxMiBAQCBERUNMQVJFX0FMTE9DQVRPUl9XSVRIX0hFQVBfSURFTlRJRklFUihIYXNoVGFi bGUpOwogICAgIHRlbXBsYXRlPHR5cGVuYW1lIEhhc2hUcmFuc2xhdG9yLCB0eXBlbmFtZSBUPgog ICAgIEFMV0FZU19JTkxJTkUgYXV0byBIYXNoVGFibGU8S2V5LCBWYWx1ZSwgRXh0cmFjdG9yLCBI YXNoRnVuY3Rpb25zLCBUcmFpdHMsIEtleVRyYWl0cz46OmlubGluZUxvb2t1cChjb25zdCBUJiBr ZXkpIC0+IFZhbHVlVHlwZSoKICAgICB7CisjaWYgVVNFKEFOR0xFKQogICAgICAgICBzdGF0aWNf YXNzZXJ0KHNpemVvZihLZXkpICsgc2l6ZW9mKFZhbHVlKSA8IDI1MCwgIllvdXIgSGFzaFRhYmxl IHR5cGVzIGFyZSB0b28gYmlnIHRvIGVmZmljaWVudGx5IG1vdmUgd2hlbiByZWhhc2hpbmcuICBD b25zaWRlciB1c2luZyBVbmlxdWVSZWYgaW5zdGVhZCIpOwotCisjZWxzZQorICAgICAgICAvLyBG SVhNRTogaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIyMzYzNworICAg ICAgICBzdGF0aWNfYXNzZXJ0KHNpemVvZihLZXkpICsgc2l6ZW9mKFZhbHVlKSA8IDQwMCwgIllv dXIgSGFzaFRhYmxlIHR5cGVzIGFyZSB0b28gYmlnIHRvIGVmZmljaWVudGx5IG1vdmUgd2hlbiBy ZWhhc2hpbmcuICBDb25zaWRlciB1c2luZyBVbmlxdWVSZWYgaW5zdGVhZCIpOworI2VuZGlmCiAg ICAgICAgIGNoZWNrS2V5PEhhc2hUcmFuc2xhdG9yPihrZXkpOwogCiAgICAgICAgIHVuc2lnbmVk IGsgPSAwOwo=