74498 2011-12-14 04:24:34 -0800 ensure consistent heuristics 2011-12-21 14:30:32 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified UNCONFIRMED P2 Normal --- 72938 72960 0 wingo webkit-unassigned barraclough eric fpizlo ggaren oliver oldest_to_newest 520844 0 wingo 2011-12-14 04:24:34 -0800 Patch to be attached factors consistency-checking routine out of initializeOptions, née initializeHeuristics, and clamps various values at runtime. Depends on bug 72938. 520853 1 119203 wingo 2011-12-14 04:48:04 -0800 Created attachment 119203 Patch 525454 2 eric 2011-12-21 12:18:20 -0800 It's best to CC the JSC guys if you want this reviewed. :) 525461 3 119203 mrobinson 2011-12-21 12:24:21 -0800 Comment on attachment 119203 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=119203&action=review > Source/JavaScriptCore/runtime/Options.cpp:51 > +#define CLAMP(var, min, max) var = ((var <= min) ? min : ((var >= max) ? max : var)) Perhaps it's possible to use clampTo from MathExtras.h here? It looks suspiciously like it already handles numeric_limits minimums, but I don't know if it has support for int32_t yet. 525599 4 119203 fpizlo 2011-12-21 14:30:32 -0800 Comment on attachment 119203 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=119203&action=review > Source/JavaScriptCore/runtime/Options.cpp:55 > + executionCounterValueForOptimizeNextInvocation = 0; // Not configurable. That's not right. We want to be able to change this. > Source/JavaScriptCore/runtime/Options.cpp:56 > + CLAMP(executionCounterValueForOptimizeSoon, std::numeric_limits<int32_t>::min(), executionCounterValueForOptimizeNextInvocation - 1); That's probably a bit too strict. Could drop the - 1. > Source/JavaScriptCore/runtime/Options.cpp:65 > + CLAMP(desiredProfileFullnessRate, 0, desiredProfileLivenessRate); Why? These two nunbers are unrelated. > Source/JavaScriptCore/runtime/Options.cpp:66 > + CLAMP(numberOfGCMarkers, 1, 8); Not sure about this. On Mac the default is to top out at 4 markers *by default*, but we want to enable more markers. What's wrong with having, say, 24 markers? My machine has 24 logical CPUs. > Source/JavaScriptCore/runtime/Options.cpp:82 > + ASSERT((static_cast<int64_t>(executionCounterValueForOptimizeAfterLongWarmUp) << reoptimizationRetryCounterMax) >= static_cast<int64_t>(std::numeric_limits<int32_t>::min())); I think that CheckedArithmetic.h might be better than this stuff. > Source/JavaScriptCore/runtime/Options.h:68 > + m(unsigned, reoptimizationRetryCounterMax, 30) \ Why set this to 30 and then recompute it anyway? 119203 2011-12-14 04:48:04 -0800 2011-12-21 14:30:32 -0800 Patch bug-74498-20111214134802.patch text/plain 6239 wingo U3VidmVyc2lvbiBSZXZpc2lvbjogMTAyNzYyCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCBk MDQ4NzE3ZjBkYzI3MTI0NjdmODJmYWExODEyZjcxOWJjMzJkYzAzLi4xNTBjNzcyMTQ0YTIwOTNl ZmM2OGFkMmE1MTI1ZmM4MzRmYzg0YWYyIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNyBAQAorMjAxMS0xMi0xNCAgQW5keSBXaW5nbyAgPHdpbmdvQGlnYWxpYS5jb20+CisK KyAgICAgICAgRW5zdXJlIGNvbnNpc3RlbnQgaGV1cmlzdGljcworICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NzQ0OTgKKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIHJ1bnRpbWUvT3B0aW9ucy5jcHAgKEpTQzo6 T3B0aW9uczo6ZW5zdXJlQ29uc2lzdGVuY3kpOgorICAgICAgICAoSlNDOjpPcHRpb25zOjppbml0 aWFsaXplT3B0aW9ucyk6IE1ha2Ugc3VyZSBvcHRpb25zIGFyZQorICAgICAgICBjb25zaXN0ZW50 LCBldmVuIGluIG5vbi1kZWJ1ZyBtb2RlLgorICAgICAgICAqIHJ1bnRpbWUvT3B0aW9ucy5oOiBE ZWZhdWx0IHRoZSByZW9wdGltaXphdGlvblJldHJ5Q291bnRlciB0byBhCisgICAgICAgIGhpZ2gg dmFsdWUgb3V0IG9mIHJhbmdlLCBhbmQgcmVseSBvbiBlbnN1cmVDb25zaXN0ZW5jeSB0byBwdWxs IGl0CisgICAgICAgIGRvd24uCisKIDIwMTEtMTEtMjkgIEFuZHkgV2luZ28gIDx3aW5nb0BpZ2Fs aWEuY29tPgogCiAgICAgICAgIFVzZSBtb3JlIG1hY3JvbG9neSBpbiBKU0M6Ok9wdGlvbnMKZGlm ZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL09wdGlvbnMuY3BwIGIvU291 cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvT3B0aW9ucy5jcHAKaW5kZXggODk5ZjY2ZWNiYTY3 YmU3YWVkMGM2YWMxZDBiZTk2NTg5YTAyZTcyZi4uYjhjYTFlYzE1ZmVhM2RiZjhmYWMxYmFkMTcx MTYyODRjYjEwNTAwZiAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUv T3B0aW9ucy5jcHAKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvT3B0aW9ucy5j cHAKQEAgLTQ4LDYgKzQ4LDQwIEBAIG5hbWVzcGFjZSBKU0MgeyBuYW1lc3BhY2UgT3B0aW9ucyB7 CiBGT1JfRUFDSF9PUFRJT04oREVGSU5FKQogI3VuZGVmIERFRklORQogCisjZGVmaW5lIENMQU1Q KHZhciwgbWluLCBtYXgpIHZhciA9ICgodmFyIDw9IG1pbikgPyBtaW4gOiAoKHZhciA+PSBtYXgp ID8gbWF4IDogdmFyKSkKKworc3RhdGljIHZvaWQgZW5zdXJlQ29uc2lzdGVuY3koKQoreworICAg IGV4ZWN1dGlvbkNvdW50ZXJWYWx1ZUZvck9wdGltaXplTmV4dEludm9jYXRpb24gPSAwOyAvLyBO b3QgY29uZmlndXJhYmxlLgorICAgIENMQU1QKGV4ZWN1dGlvbkNvdW50ZXJWYWx1ZUZvck9wdGlt aXplU29vbiwgc3RkOjpudW1lcmljX2xpbWl0czxpbnQzMl90Pjo6bWluKCksIGV4ZWN1dGlvbkNv dW50ZXJWYWx1ZUZvck9wdGltaXplTmV4dEludm9jYXRpb24gLSAxKTsKKyAgICBDTEFNUChleGVj dXRpb25Db3VudGVyVmFsdWVGb3JPcHRpbWl6ZUFmdGVyV2FybVVwLCBzdGQ6Om51bWVyaWNfbGlt aXRzPGludDMyX3Q+OjptaW4oKSwgZXhlY3V0aW9uQ291bnRlclZhbHVlRm9yT3B0aW1pemVTb29u KTsKKyAgICBDTEFNUChleGVjdXRpb25Db3VudGVyVmFsdWVGb3JPcHRpbWl6ZUFmdGVyTG9uZ1dh cm1VcCwgc3RkOjpudW1lcmljX2xpbWl0czxpbnQzMl90Pjo6bWluKCksIGV4ZWN1dGlvbkNvdW50 ZXJWYWx1ZUZvck9wdGltaXplQWZ0ZXJXYXJtVXApOworICAgIENMQU1QKGV4ZWN1dGlvbkNvdW50 ZXJWYWx1ZUZvckRvbnRPcHRpbWl6ZUFueXRpbWVTb29uLCBzdGQ6Om51bWVyaWNfbGltaXRzPGlu dDMyX3Q+OjptaW4oKSwgZXhlY3V0aW9uQ291bnRlclZhbHVlRm9yT3B0aW1pemVBZnRlckxvbmdX YXJtVXApOworICAgIENMQU1QKGxpa2VseVRvVGFrZVNsb3dDYXNlVGhyZXNob2xkLCAwLjAsIDEu MCk7CisgICAgQ0xBTVAoY291bGRUYWtlU2xvd0Nhc2VUaHJlc2hvbGQsIDAuMCwgbGlrZWx5VG9U YWtlU2xvd0Nhc2VUaHJlc2hvbGQpOworICAgIENMQU1QKGNvdWxkVGFrZVNsb3dDYXNlTWluaW11 bUNvdW50LCAwLCBsaWtlbHlUb1Rha2VTbG93Q2FzZU1pbmltdW1Db3VudCk7CisgICAgQ0xBTVAo bWluaW11bU9wdGltaXphdGlvbkRlbGF5LCAwLCBtYXhpbXVtT3B0aW1pemF0aW9uRGVsYXkpOwor ICAgIENMQU1QKGRlc2lyZWRQcm9maWxlTGl2ZW5lc3NSYXRlLCAwLCAxLjApOworICAgIENMQU1Q KGRlc2lyZWRQcm9maWxlRnVsbG5lc3NSYXRlLCAwLCBkZXNpcmVkUHJvZmlsZUxpdmVuZXNzUmF0 ZSk7CisgICAgQ0xBTVAobnVtYmVyT2ZHQ01hcmtlcnMsIDEsIDgpOworCisgICAgQVNTRVJUKGV4 ZWN1dGlvbkNvdW50ZXJWYWx1ZUZvckRvbnRPcHRpbWl6ZUFueXRpbWVTb29uIDw9IGV4ZWN1dGlv bkNvdW50ZXJWYWx1ZUZvck9wdGltaXplQWZ0ZXJMb25nV2FybVVwKTsKKyAgICBBU1NFUlQoZXhl Y3V0aW9uQ291bnRlclZhbHVlRm9yT3B0aW1pemVBZnRlckxvbmdXYXJtVXAgPD0gZXhlY3V0aW9u Q291bnRlclZhbHVlRm9yT3B0aW1pemVBZnRlcldhcm1VcCk7CisgICAgQVNTRVJUKGV4ZWN1dGlv bkNvdW50ZXJWYWx1ZUZvck9wdGltaXplQWZ0ZXJXYXJtVXAgPD0gZXhlY3V0aW9uQ291bnRlclZh bHVlRm9yT3B0aW1pemVTb29uKTsKKyAgICBBU1NFUlQoZXhlY3V0aW9uQ291bnRlclZhbHVlRm9y T3B0aW1pemVBZnRlcldhcm1VcCA8IDApOworICAgIEFTU0VSVChleGVjdXRpb25Db3VudGVyVmFs dWVGb3JPcHRpbWl6ZVNvb24gPD0gZXhlY3V0aW9uQ291bnRlclZhbHVlRm9yT3B0aW1pemVOZXh0 SW52b2NhdGlvbik7CisgICAgCisgICAgLy8gRW5zdXJlIHRoYXQgdGhlIGV4ZWN1dGUgY291bnRl ciBkb2Vzbid0IG92ZXJmbG93IHdoZW4gdXNpbmcgaXQgdG8KKyAgICAvLyBsZWZ0LXNoaWZ0IHRo ZSBleGVjdXRpb24gY291bnRlciBieSB0aGUgcmVvcHRpbWl6YXRpb24gcmV0cnkgY291bnRlci4K KyAgICAvLyBDdXJyZW50bHkgdGhlIHZhbHVlIGVuZHMgdXAgYmVpbmcgMTgsIHNvIHRoaXMgbG9v cCBpcyBub3Qgc28gdGVycmlibGU7IGl0CisgICAgLy8gcHJvYmFibHkgdGFrZXMgdXAgfjEwMCBj eWNsZXMgdG90YWwgb24gYSAzMi1iaXQgcHJvY2Vzc29yLgorICAgIHdoaWxlICgoc3RhdGljX2Nh c3Q8aW50NjRfdD4oZXhlY3V0aW9uQ291bnRlclZhbHVlRm9yT3B0aW1pemVBZnRlckxvbmdXYXJt VXApIDw8IChyZW9wdGltaXphdGlvblJldHJ5Q291bnRlck1heCkpIDwgc3RhdGljX2Nhc3Q8aW50 NjRfdD4oc3RkOjpudW1lcmljX2xpbWl0czxpbnQzMl90Pjo6bWluKCkpKQorICAgICAgICByZW9w dGltaXphdGlvblJldHJ5Q291bnRlck1heC0tOworICAgIAorICAgIEFTU0VSVCgoc3RhdGljX2Nh c3Q8aW50NjRfdD4oZXhlY3V0aW9uQ291bnRlclZhbHVlRm9yT3B0aW1pemVBZnRlckxvbmdXYXJt VXApIDw8IHJlb3B0aW1pemF0aW9uUmV0cnlDb3VudGVyTWF4KSA8IDApOworICAgIEFTU0VSVCgo c3RhdGljX2Nhc3Q8aW50NjRfdD4oZXhlY3V0aW9uQ291bnRlclZhbHVlRm9yT3B0aW1pemVBZnRl ckxvbmdXYXJtVXApIDw8IHJlb3B0aW1pemF0aW9uUmV0cnlDb3VudGVyTWF4KSA+PSBzdGF0aWNf Y2FzdDxpbnQ2NF90PihzdGQ6Om51bWVyaWNfbGltaXRzPGludDMyX3Q+OjptaW4oKSkpOworfQor CiAjaWYgRU5BQkxFKFJVTl9USU1FX0hFVVJJU1RJQ1MpCiBzdGF0aWMgYm9vbCBwYXJzZShjb25z dCBjaGFyKiBzdHJpbmcsIGludDMyX3QmIHZhbHVlKQogewpAQCAtMTE2LDIzICsxNTAsNyBAQCB2 b2lkIGluaXRpYWxpemVPcHRpb25zKCkKICAgICAgICAgbnVtYmVyT2ZHQ01hcmtlcnMgPSBjcHVz VG9Vc2U7CiAgICAgfQogICAgIAotICAgIEFTU0VSVChleGVjdXRpb25Db3VudGVyVmFsdWVGb3JE b250T3B0aW1pemVBbnl0aW1lU29vbiA8PSBleGVjdXRpb25Db3VudGVyVmFsdWVGb3JPcHRpbWl6 ZUFmdGVyTG9uZ1dhcm1VcCk7Ci0gICAgQVNTRVJUKGV4ZWN1dGlvbkNvdW50ZXJWYWx1ZUZvck9w dGltaXplQWZ0ZXJMb25nV2FybVVwIDw9IGV4ZWN1dGlvbkNvdW50ZXJWYWx1ZUZvck9wdGltaXpl QWZ0ZXJXYXJtVXApOwotICAgIEFTU0VSVChleGVjdXRpb25Db3VudGVyVmFsdWVGb3JPcHRpbWl6 ZUFmdGVyV2FybVVwIDw9IGV4ZWN1dGlvbkNvdW50ZXJWYWx1ZUZvck9wdGltaXplU29vbik7Ci0g ICAgQVNTRVJUKGV4ZWN1dGlvbkNvdW50ZXJWYWx1ZUZvck9wdGltaXplQWZ0ZXJXYXJtVXAgPCAw KTsKLSAgICBBU1NFUlQoZXhlY3V0aW9uQ291bnRlclZhbHVlRm9yT3B0aW1pemVTb29uIDw9IGV4 ZWN1dGlvbkNvdW50ZXJWYWx1ZUZvck9wdGltaXplTmV4dEludm9jYXRpb24pOwotICAgIAotICAg IC8vIENvbXB1dGUgdGhlIG1heGltdW0gdmFsdWUgb2YgdGhlIHJlb3B0aW1pemF0aW9uIHJldHJ5 IGNvdW50ZXIuIFRoaXMgaXMgc2ltcGx5Ci0gICAgLy8gdGhlIGxhcmdlc3QgdmFsdWUgYXQgd2hp Y2ggd2UgZG9uJ3Qgb3ZlcmZsb3cgdGhlIGV4ZWN1dGUgY291bnRlciwgd2hlbiB1c2luZyBpdAot ICAgIC8vIHRvIGxlZnQtc2hpZnQgdGhlIGV4ZWN1dGlvbiBjb3VudGVyIGJ5IHRoaXMgYW1vdW50 LiBDdXJyZW50bHkgdGhlIHZhbHVlIGVuZHMKLSAgICAvLyB1cCBiZWluZyAxOCwgc28gdGhpcyBs b29wIGlzIG5vdCBzbyB0ZXJyaWJsZTsgaXQgcHJvYmFibHkgdGFrZXMgdXAgfjEwMCBjeWNsZXMK LSAgICAvLyB0b3RhbCBvbiBhIDMyLWJpdCBwcm9jZXNzb3IuCi0gICAgcmVvcHRpbWl6YXRpb25S ZXRyeUNvdW50ZXJNYXggPSAwOwotICAgIHdoaWxlICgoc3RhdGljX2Nhc3Q8aW50NjRfdD4oZXhl Y3V0aW9uQ291bnRlclZhbHVlRm9yT3B0aW1pemVBZnRlckxvbmdXYXJtVXApIDw8IChyZW9wdGlt aXphdGlvblJldHJ5Q291bnRlck1heCArIDEpKSA+PSBzdGF0aWNfY2FzdDxpbnQ2NF90PihzdGQ6 Om51bWVyaWNfbGltaXRzPGludDMyX3Q+OjptaW4oKSkpCi0gICAgICAgIHJlb3B0aW1pemF0aW9u UmV0cnlDb3VudGVyTWF4Kys7Ci0gICAgCi0gICAgQVNTRVJUKChzdGF0aWNfY2FzdDxpbnQ2NF90 PihleGVjdXRpb25Db3VudGVyVmFsdWVGb3JPcHRpbWl6ZUFmdGVyTG9uZ1dhcm1VcCkgPDwgcmVv cHRpbWl6YXRpb25SZXRyeUNvdW50ZXJNYXgpIDwgMCk7Ci0gICAgQVNTRVJUKChzdGF0aWNfY2Fz dDxpbnQ2NF90PihleGVjdXRpb25Db3VudGVyVmFsdWVGb3JPcHRpbWl6ZUFmdGVyTG9uZ1dhcm1V cCkgPDwgcmVvcHRpbWl6YXRpb25SZXRyeUNvdW50ZXJNYXgpID49IHN0YXRpY19jYXN0PGludDY0 X3Q+KHN0ZDo6bnVtZXJpY19saW1pdHM8aW50MzJfdD46Om1pbigpKSk7CisgICAgZW5zdXJlQ29u c2lzdGVuY3koKTsKIH0KIAogfSB9IC8vIG5hbWVzcGFjZSBKU0M6Ok9wdGlvbnMKZGlmZiAtLWdp dCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL09wdGlvbnMuaCBiL1NvdXJjZS9KYXZh U2NyaXB0Q29yZS9ydW50aW1lL09wdGlvbnMuaAppbmRleCAyZjgwYmY2OTJkNjRmOGYzNzIyNzY1 ZTBlNzBkOTYxZjYxZmJmYzRjLi42YzFmYjA5OTA5YzUzNjVhYmZkNDgzNTViMjZjYWY1MjZiMTI0 NmViIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9PcHRpb25zLmgK KysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvT3B0aW9ucy5oCkBAIC02NSw3ICs2 NSw3IEBAIG5hbWVzcGFjZSBKU0MgeyBuYW1lc3BhY2UgT3B0aW9ucyB7CiAgICAgbSh1bnNpZ25l ZCwgbGFyZ2VGYWlsQ291bnRUaHJlc2hvbGRCYXNlLCAgICAgICAgICAgMjApIFwKICAgICBtKHVu c2lnbmVkLCBsYXJnZUZhaWxDb3VudFRocmVzaG9sZEJhc2VGb3JMb29wLCAgICAxKSBcCiAgICAg XAotICAgIG0odW5zaWduZWQsIHJlb3B0aW1pemF0aW9uUmV0cnlDb3VudGVyTWF4LCAgICAgICAg IDApIFwKKyAgICBtKHVuc2lnbmVkLCByZW9wdGltaXphdGlvblJldHJ5Q291bnRlck1heCwgICAg ICAgICAzMCkgXAogICAgIG0odW5zaWduZWQsIHJlb3B0aW1pemF0aW9uUmV0cnlDb3VudGVyU3Rl cCwgICAgICAgIDEpIFwKICAgICBcCiAgICAgbSh1bnNpZ25lZCwgbWluaW11bU9wdGltaXphdGlv bkRlbGF5LCAgICAgICAgICAgICAgMSkgXAo=