65840 2011-08-07 23:17:21 -0700 Fix comment in NewSpace.h 2022-02-27 23:23:11 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED INVALID P2 Normal --- 65458 1 zherczeg webkit-unassigned barraclough ggaren oliver oldest_to_newest 447846 0 zherczeg 2011-08-07 23:17:21 -0700 Wrong comments can be misleading when you need to fix a bug. 447848 1 103203 zherczeg 2011-08-07 23:23:30 -0700 Created attachment 103203 patch 448154 2 barraclough 2011-08-08 12:08:24 -0700 cc'ing GC experts 448331 3 103203 ggaren 2011-08-08 15:56:48 -0700 Comment on attachment 103203 patch View in context: https://bugs.webkit.org/attachment.cgi?id=103203&action=review > Source/JavaScriptCore/heap/NewSpace.h:88 > + // [ 128 - preciseStep, 256... 1024 ) I don't think this is true: the first imprecise size class is exactly 128, not 128 - preciseStep. 448485 4 zherczeg 2011-08-09 02:36:31 -0700 > I don't think this is true: the first imprecise size class is exactly 128, not 128 - preciseStep. See bug 65458 (Depends on). The bug was that an allocation of 103 bytes is bigger than 95, which is the end of last precise class on 64 bit. So it is allocated from the first inprecise class. Or should it go to a precise class? 448711 5 ggaren 2011-08-09 11:49:45 -0700 > See bug 65458 (Depends on). The bug was that an allocation of 103 bytes is bigger than 95, which is the end of last precise class on 64 bit. So it is allocated from the first inprecise class. Or should it go to a precise class? Where to allocate is one question. Independently, this comment describes the size of the cells in the class. That size is 128, not 128 - preciseStep. 448995 6 zherczeg 2011-08-09 23:29:54 -0700 > Where to allocate is one question. Independently, this comment describes the size of the cells in the class. That size is 128, not 128 - preciseStep. Are you sure? The comment said: // [ 128, 256... 1024 ) The size of the first block is really 128, but the second is 128 as well, not 256, and the last is 128 as well, not 1024. Instead I think the comment describes the ranges, the first range is [128-256), the second is [256-384) ... The growth rate is clearly linear: static const size_t preciseCutoff = 128; static const size_t impreciseStep = preciseCutoff; return m_impreciseSizeClasses[(bytes - 1) / impreciseStep]; 449438 7 ggaren 2011-08-10 15:32:15 -0700 (gdb) disp cellSize (gdb) run [Switching to process 3491] Running… 3: cellSize = 32 (gdb) c Continuing. 3: cellSize = 64 (gdb) Continuing. 3: cellSize = 96 (gdb) Continuing. 4: cellSize = 128 (gdb) Continuing. 4: cellSize = 256 (gdb) Continuing. 4: cellSize = 384 (gdb) Continuing. 4: cellSize = 512 (gdb) Continuing. 4: cellSize = 640 (gdb) Continuing. 4: cellSize = 768 (gdb) Continuing. 4: cellSize = 896 449621 8 zherczeg 2011-08-10 23:45:07 -0700 Oh, those brackets were confused me. [a,b) in mathematics means a range of numbers, where x >= a and x < b. Sorry for the misunderstanding. 103203 2011-08-07 23:23:30 -0700 2022-02-27 23:23:11 -0800 patch 0001-comment-fix.patch text/plain 1981 zherczeg RnJvbSA4YmU1MWIxZWRlNTQ0OTczZjVkMGY4NGY0MWFmYmU4MGQ3YjFiNDUzIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBab2x0YW4gSGVyY3plZyA8emhlcmN6ZWdAaW5mLnUtc3plZ2Vk Lmh1PgpEYXRlOiBTdW4sIDcgQXVnIDIwMTEgMjM6MjE6MjIgLTA3MDAKU3ViamVjdDogW1BBVENI XSBjb21tZW50LWZpeAoKLS0tCiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nICAgICAg IHwgICAxMSArKysrKysrKysrKwogU291cmNlL0phdmFTY3JpcHRDb3JlL2hlYXAvTmV3U3BhY2Uu aCB8ICAgIDUgKysrLS0KIDIgZmlsZXMgY2hhbmdlZCwgMTQgaW5zZXJ0aW9ucygrKSwgMiBkZWxl dGlvbnMoLSkKCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nIGIv U291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCBkMThiZmI2Li4wMjg0YjU1IDEw MDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9K YXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNCBAQAorMjAxMS0wOC0wNyAgWm9s dGFuIEhlcmN6ZWcgIDx6aGVyY3plZ0BpbmYudS1zemVnZWQuaHU+CisKKyAgICAgICAgRml4IGNv bW1lbnQgaW4gTmV3U3BhY2UuaAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9NjU4NDAKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICBUaGUgY29tbWVudCBzaG91bGQgc2hvdyB0aGUgcHJvcGVyIHNpemUgYW5kIG51 bWJlciBvZiBzbG90cy4KKworICAgICAgICAqIGhlYXAvTmV3U3BhY2UuaDoKKwogMjAxMS0wOC0w NyAgTWFydGluIFJvYmluc29uICA8bXJvYmluc29uQGlnYWxpYS5jb20+CiAKICAgICAgICAgRGlz dHJpYnV0aW9uIGZpeCBmb3IgR1RLKy4KZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29y ZS9oZWFwL05ld1NwYWNlLmggYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvaGVhcC9OZXdTcGFjZS5o CmluZGV4IGIwZTk0MzEuLjhlM2FhMGEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29y ZS9oZWFwL05ld1NwYWNlLmgKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL2hlYXAvTmV3U3Bh Y2UuaApAQCAtNzgsMTMgKzc4LDE0IEBAIG5hbWVzcGFjZSBKU0MgewogICAgICAgICB0ZW1wbGF0 ZTx0eXBlbmFtZSBGdW5jdG9yPiB0eXBlbmFtZSBGdW5jdG9yOjpSZXR1cm5UeXBlIGZvckVhY2hC bG9jaygpOwogCiAgICAgcHJpdmF0ZToKLSAgICAgICAgLy8gWyA4LCAxNi4uLiAxMjggKQorICAg ICAgICAvLyAzMiBiaXQ6IFsgMTYsIDMyLCA0OCAuLi4gMTI4IC0gMTYgKQorICAgICAgICAvLyA2 NCBiaXQ6IFsgMzIsIDY0LCA5NiA9IDEyOCAtIDMyICkKICAgICAgICAgc3RhdGljIGNvbnN0IHNp emVfdCBwcmVjaXNlU3RlcCA9IE1hcmtlZEJsb2NrOjphdG9tU2l6ZTsKICAgICAgICAgc3RhdGlj IGNvbnN0IHNpemVfdCBwcmVjaXNlQ3V0b2ZmID0gMTI4OwogICAgICAgICBzdGF0aWMgY29uc3Qg c2l6ZV90IG1heGltdW1QcmVjaXNlQWxsb2NhdGlvblNpemUgPSBwcmVjaXNlQ3V0b2ZmIC0gcHJl Y2lzZVN0ZXA7CiAgICAgICAgIHN0YXRpYyBjb25zdCBzaXplX3QgcHJlY2lzZUNvdW50ID0gcHJl Y2lzZUN1dG9mZiAvIHByZWNpc2VTdGVwIC0gMTsKIAotICAgICAgICAvLyBbIDEyOCwgMjU2Li4u IDEwMjQgKQorICAgICAgICAvLyBbIDEyOCAtIHByZWNpc2VTdGVwLCAyNTYuLi4gMTAyNCApCiAg ICAgICAgIHN0YXRpYyBjb25zdCBzaXplX3QgaW1wcmVjaXNlU3RlcCA9IHByZWNpc2VDdXRvZmY7 CiAgICAgICAgIHN0YXRpYyBjb25zdCBzaXplX3QgaW1wcmVjaXNlQ3V0b2ZmID0gbWF4Q2VsbFNp emU7CiAgICAgICAgIHN0YXRpYyBjb25zdCBzaXplX3QgaW1wcmVjaXNlQ291bnQgPSBpbXByZWNp c2VDdXRvZmYgLyBpbXByZWNpc2VTdGVwIC0gMTsKLS0gCjEuNy4yLjMKCg==