138549 2014-11-09 14:50:47 -0800 Remove BloomFilter size limit 2022-10-10 09:49:53 -0700 1 1 1 Unclassified WebKit Web Template Framework 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 koivisto webkit-unassigned ahmad.saleem792 benjamin cdumez cmarcelo commit-queue oldest_to_newest 1047413 0 koivisto 2014-11-09 14:50:47 -0800 It works fine with sizes greater than 2^16. 1047416 1 241261 koivisto 2014-11-09 14:53:46 -0800 Created attachment 241261 patch 1047419 2 241261 cdumez 2014-11-09 15:18:46 -0800 Comment on attachment 241261 patch View in context: https://bugs.webkit.org/attachment.cgi?id=241261&action=review > Source/WTF/wtf/BloomFilter.h:-40 > - static_assert(keyBits <= 16, "BloomFilter key size must be less than or equal to 16!"); Shouldn't we make sure it is at least <= 32 so that 1 << keyBits below doesn't overflow? I have looked carefully at the implementation but was this 16 value somehow corrected to the 16 value used in secondSlot() below? 1047420 3 cdumez 2014-11-09 15:26:40 -0800 (In reply to comment #2) > Comment on attachment 241261 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=241261&action=review > > > Source/WTF/wtf/BloomFilter.h:-40 > > - static_assert(keyBits <= 16, "BloomFilter key size must be less than or equal to 16!"); > > Shouldn't we make sure it is at least <= 32 so that 1 << keyBits below > doesn't overflow? > > I have looked carefully at the implementation but was this 16 value somehow > corrected to the 16 value used in secondSlot() below? typo: s/corrected/connected 1047424 4 koivisto 2014-11-09 15:38:20 -0800 I just removed the assert as it is unlikely anyone would try to create anything close to a 4GB filter. I think we get sensible slot spread without any code changes. It is not entirely symmetric anymore, first slot uses the entire table while second is the lowest 2^16 slots. However there are only 16 unique bits per slot in the key anyway. 1047542 5 241261 kling 2014-11-10 08:59:16 -0800 Comment on attachment 241261 patch Sure, r=me 1904515 6 ahmad.saleem792 2022-10-10 09:49:53 -0700 Landed - https://github.com/WebKit/WebKit/commit/c728c9b09cebb25ccf8b6d7906be1ccb334ac4e2 and didn't backed out. Marking "RESOLVED FIXED". 241261 2014-11-09 14:53:46 -0800 2014-11-10 08:59:16 -0800 patch bloom-filter-size.patch text/plain 1176 koivisto SW5kZXg6IFNvdXJjZS9XVEYvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XVEYvQ2hh bmdlTG9nCShyZXZpc2lvbiAxNzU3OTUpCisrKyBTb3VyY2UvV1RGL0NoYW5nZUxvZwkod29ya2lu ZyBjb3B5KQpAQCAtMSwzICsxLDEyIEBACisyMDE0LTExLTA5ICBBbnR0aSBLb2l2aXN0byAgPGFu dHRpQGFwcGxlLmNvbT4KKworICAgICAgICBSZW1vdmUgQmxvb21GaWx0ZXIgc2l6ZSBsaW1pdAor ICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTM4NTQ5CisK KyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiB3dGYvQmxv b21GaWx0ZXIuaDogUmVtb3ZlIHRoZSBzdGF0aWNfYXNzZXJ0LCB0aGUgY29kZSB3b3JrcyBmaW5l IHdpdGggbGFyZ2VyIHNpemVzLgorCiAyMDE0LTExLTA4ICBEYXJpbiBBZGxlciAgPGRhcmluQGFw cGxlLmNvbT4KIAogICAgICAgICBSZXBsYWNlIEZpbGVUaHJlYWQgY2xhc3Mgd2l0aCBhIHNpbmds ZSBmdW5jdGlvbgpJbmRleDogU291cmNlL1dURi93dGYvQmxvb21GaWx0ZXIuaAo9PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 Ci0tLSBTb3VyY2UvV1RGL3d0Zi9CbG9vbUZpbHRlci5oCShyZXZpc2lvbiAxNzU3NDYpCisrKyBT b3VyY2UvV1RGL3d0Zi9CbG9vbUZpbHRlci5oCSh3b3JraW5nIGNvcHkpCkBAIC0zNyw4ICszNyw2 IEBAIHRlbXBsYXRlIDx1bnNpZ25lZCBrZXlCaXRzPgogY2xhc3MgQmxvb21GaWx0ZXIgewogICAg IFdURl9NQUtFX0ZBU1RfQUxMT0NBVEVEOwogcHVibGljOgotICAgIHN0YXRpY19hc3NlcnQoa2V5 Qml0cyA8PSAxNiwgIkJsb29tRmlsdGVyIGtleSBzaXplIG11c3QgYmUgbGVzcyB0aGFuIG9yIGVx dWFsIHRvIDE2ISIpOwotCiAgICAgc3RhdGljIGNvbnN0IHNpemVfdCB0YWJsZVNpemUgPSAxIDw8 IGtleUJpdHM7CiAgICAgc3RhdGljIGNvbnN0IHVuc2lnbmVkIGtleU1hc2sgPSAoMSA8PCBrZXlC aXRzKSAtIDE7CiAgICAgc3RhdGljIHVpbnQ4X3QgbWF4aW11bUNvdW50KCkgeyByZXR1cm4gc3Rk OjpudW1lcmljX2xpbWl0czx1aW50OF90Pjo6bWF4KCk7IH0K