52222 2011-01-11 09:45:53 -0800 Microoptimization in ~JSString 2011-01-11 10:52:33 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED P2 Normal --- 1 xan.lopez webkit-unassigned xan.lopez oldest_to_newest 332409 0 xan.lopez 2011-01-11 09:45:53 -0800 ssia. 332411 1 78544 xan.lopez 2011-01-11 09:49:41 -0800 Created attachment 78544 jsstringdtor.diff Small optimization in the JSString dtor. SunSpider results: TEST COMPARISON FROM TO DETAILS ============================================================================= ** TOTAL **: 1.003x as fast 275.4ms +/- 0.1% 274.7ms +/- 0.1% significant ============================================================================= 3d: - 36.4ms +/- 0.5% 36.2ms +/- 0.5% cube: - 11.9ms +/- 1.0% 11.8ms +/- 1.0% morph: - 11.2ms +/- 0.6% 11.2ms +/- 0.6% raytrace: - 13.2ms +/- 0.5% 13.2ms +/- 0.5% access: - 38.7ms +/- 0.2% 38.7ms +/- 0.2% binary-trees: ?? 3.7ms +/- 2.0% 3.7ms +/- 2.0% not conclusive: might be *1.002x as slow* fannkuch: - 19.0ms +/- 0.1% 19.0ms +/- 0.0% nbody: - 11.0ms +/- 0.0% 11.0ms +/- 0.0% nsieve: - 5.0ms +/- 0.0% 5.0ms +/- 0.0% bitops: ?? 23.6ms +/- 0.3% 23.7ms +/- 0.3% not conclusive: might be *1.002x as slow* 3bit-bits-in-byte: - 3.0ms +/- 0.0% 3.0ms +/- 0.0% bits-in-byte: - 6.0ms +/- 0.0% 6.0ms +/- 0.0% bitwise-and: ?? 5.6ms +/- 1.4% 5.7ms +/- 1.4% not conclusive: might be *1.008x as slow* nsieve-bits: - 9.0ms +/- 0.1% 9.0ms +/- 0.0% controlflow: - 2.0ms +/- 0.0% 2.0ms +/- 0.0% recursive: - 2.0ms +/- 0.0% 2.0ms +/- 0.0% crypto: - 16.0ms +/- 0.1% 16.0ms +/- 0.1% aes: - 10.0ms +/- 0.0% 10.0ms +/- 0.0% md5: - 3.0ms +/- 0.6% 3.0ms +/- 0.6% sha1: - 3.0ms +/- 0.0% 3.0ms +/- 0.0% date: ?? 31.4ms +/- 0.3% 31.4ms +/- 0.3% not conclusive: might be *1.001x as slow* format-tofte: - 16.0ms +/- 0.2% 16.0ms +/- 0.2% format-xparb: ?? 15.3ms +/- 0.5% 15.4ms +/- 0.6% not conclusive: might be *1.003x as slow* math: - 29.0ms +/- 0.1% 29.0ms +/- 0.1% cordic: ?? 9.0ms +/- 0.3% 9.0ms +/- 0.4% not conclusive: might be *1.001x as slow* partial-sums: - 15.0ms +/- 0.0% 15.0ms +/- 0.1% spectral-norm: - 5.0ms +/- 0.0% 5.0ms +/- 0.0% regexp: ?? 12.5ms +/- 0.6% 12.6ms +/- 0.6% not conclusive: might be *1.005x as slow* dna: ?? 12.5ms +/- 0.6% 12.6ms +/- 0.6% not conclusive: might be *1.005x as slow* string: 1.008x as fast 85.8ms +/- 0.1% 85.1ms +/- 0.1% significant base64: - 9.0ms +/- 0.1% 9.0ms +/- 0.0% fasta: 1.052x as fast 12.0ms +/- 0.2% 11.4ms +/- 0.7% significant tagcloud: - 21.8ms +/- 0.3% 21.7ms +/- 0.3% unpack-code: - 32.0ms +/- 0.1% 32.0ms +/- 0.1% validate-input: - 11.0ms +/- 0.2% 11.0ms +/- 0.2% 332425 2 78544 darin 2011-01-11 10:16:27 -0800 Comment on attachment 78544 jsstringdtor.diff View in context: https://bugs.webkit.org/attachment.cgi?id=78544&action=review > Source/JavaScriptCore/runtime/JSString.h:324 > + do { > + RopeImpl::deref(m_other.m_fibers[i]); > + } while (++i < m_fiberCount); One line body, usually no braces in WebKit coding style. 332449 3 xan.lopez 2011-01-11 10:52:19 -0800 (In reply to comment #2) > (From update of attachment 78544 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=78544&action=review > > > Source/JavaScriptCore/runtime/JSString.h:324 > > + do { > > + RopeImpl::deref(m_other.m_fibers[i]); > > + } while (++i < m_fiberCount); > > One line body, usually no braces in WebKit coding style. You are right; for some reason the do without the braces seemed weird, but it goes against the style guidelines. Committed with that fix as http://trac.webkit.org/changeset/75517 332450 4 xan.lopez 2011-01-11 10:52:33 -0800 Closing. 78544 2011-01-11 09:49:41 -0800 2011-01-11 10:16:27 -0800 jsstringdtor.diff jsstringdtor.diff text/plain 3007 xan.lopez RnJvbSBkMTY3ZDY3ODdiYjJmMzI5ZTNhYWI5NTgxNWJiZDNmZjUyZTAwM2EyIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBYYW4gTG9wZXogPHhhbkBnbm9tZS5vcmc+CkRhdGU6IFR1ZSwg MTEgSmFuIDIwMTEgMTg6NDg6MzQgKzAxMDAKU3ViamVjdDogW1BBVENIXSAyMDExLTAxLTExICBY YW4gTG9wZXogIDx4bG9wZXpAaWdhbGlhLmNvbT4KCiAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZ IChPT1BTISkuCgogICAgICAgIE1pY3Jvb3B0aW1pemF0aW9uIGluIH5KU1N0cmluZwogICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD01MjIyMgoKICAgICAgICBU aGUgY2FzZSB3aGVyZSBtX2ZpYmVycyBpcyAwIHNlZW1zIHRvIGJlIHRoZSBtb3N0IGNvbW1vbiBv bmUKICAgICAgICAoYWxtb3N0IDEvMiBvZiB0aGUgdGltZSwgZm9sbG93ZWQgYXQgc29tZSBkaXN0 YW5jZSBieSBtX2ZpYmVycyA9IDEKICAgICAgICBpbiAxLzQgb2YgdGhlIGNhc2VzIGluIGEgdHlw aWNhbCBTdW5TcGlkZXIgZXhlY3V0aW9uKS4gV2UgY2FuIHNhdmUKICAgICAgICBvbmUgY29tcGFy aXNvbiBpbiB0aGlzIGNvbW1vbiBjYXNlIGJ5IGRvaW5nIGEgYml0IG9mIHJlZmFjdG9yaW5nCiAg ICAgICAgaW4gdGhlIEpTU3RyaW5nIGRlc3RydWN0b3I7IG92ZXJhbGwgYSAwLjMlIHByb2dyZXNz aW9uLCBidXQgb25seQogICAgICAgIHRoZSBzdHJpbmcgdGVzdHMgc2hvdyBpbXByb3ZlbWVudC4K CiAgICAgICAgKiBydW50aW1lL0pTU3RyaW5nLmg6CiAgICAgICAgKEpTQzo6Um9wZUJ1aWxkZXI6 On5KU1N0cmluZyk6Ci0tLQogU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZyAgICAgICAg ICB8ICAgMTcgKysrKysrKysrKysrKysrKysKIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1l L0pTU3RyaW5nLmggfCAgIDE0ICsrKysrKysrKy0tLS0tCiAyIGZpbGVzIGNoYW5nZWQsIDI2IGlu c2VydGlvbnMoKyksIDUgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvU291cmNlL0phdmFTY3Jp cHRDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKaW5kZXgg ZDRkMjFmNy4uYjQzMmNlNSAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5n ZUxvZworKysgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjAg QEAKKzIwMTEtMDEtMTEgIFhhbiBMb3BleiAgPHhsb3BlekBpZ2FsaWEuY29tPgorCisgICAgICAg IFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIE1pY3Jvb3B0aW1pemF0aW9u IGluIH5KU1N0cmluZworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9NTIyMjIKKworICAgICAgICBUaGUgY2FzZSB3aGVyZSBtX2ZpYmVycyBpcyAwIHNlZW1z IHRvIGJlIHRoZSBtb3N0IGNvbW1vbiBvbmUKKyAgICAgICAgKGFsbW9zdCAxLzIgb2YgdGhlIHRp bWUsIGZvbGxvd2VkIGF0IHNvbWUgZGlzdGFuY2UgYnkgbV9maWJlcnMgPSAxCisgICAgICAgIGlu IDEvNCBvZiB0aGUgY2FzZXMgaW4gYSB0eXBpY2FsIFN1blNwaWRlciBleGVjdXRpb24pLiBXZSBj YW4gc2F2ZQorICAgICAgICBvbmUgY29tcGFyaXNvbiBpbiB0aGlzIGNvbW1vbiBjYXNlIGJ5IGRv aW5nIGEgYml0IG9mIHJlZmFjdG9yaW5nCisgICAgICAgIGluIHRoZSBKU1N0cmluZyBkZXN0cnVj dG9yOyBvdmVyYWxsIGEgMC4zJSBwcm9ncmVzc2lvbiwgYnV0IG9ubHkKKyAgICAgICAgdGhlIHN0 cmluZyB0ZXN0cyBzaG93IGltcHJvdmVtZW50LgorCisgICAgICAgICogcnVudGltZS9KU1N0cmlu Zy5oOgorICAgICAgICAoSlNDOjpSb3BlQnVpbGRlcjo6fkpTU3RyaW5nKToKKwogMjAxMS0wMS0x MCAgRGFuaWVsIEJhdGVzICA8ZGJhdGVzQHJpbS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkg TWFydGluIFJvYmluc29uLgpkaWZmIC0tZ2l0IGEvU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRp bWUvSlNTdHJpbmcuaCBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0pTU3RyaW5nLmgK aW5kZXggYmU5Zjg4Mi4uOTViYzQzNyAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3Jl L3J1bnRpbWUvSlNTdHJpbmcuaAorKysgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9K U1N0cmluZy5oCkBAIC0zMTQsMTEgKzMxNCwxNSBAQCBuYW1lc3BhY2UgSlNDIHsKICAgICAgICAg fkpTU3RyaW5nKCkKICAgICAgICAgewogICAgICAgICAgICAgQVNTRVJUKHZwdHIoKSA9PSBKU0ds b2JhbERhdGE6OmpzU3RyaW5nVlB0cik7Ci0gICAgICAgICAgICBmb3IgKHVuc2lnbmVkIGkgPSAw OyBpIDwgbV9maWJlckNvdW50OyArK2kpCi0gICAgICAgICAgICAgICAgUm9wZUltcGw6OmRlcmVm KG1fb3RoZXIubV9maWJlcnNbaV0pOwotCi0gICAgICAgICAgICBpZiAoIW1fZmliZXJDb3VudCAm JiBtX290aGVyLm1fZmluYWxpemVyQ2FsbGJhY2spCi0gICAgICAgICAgICAgICAgbV9vdGhlci5t X2ZpbmFsaXplckNhbGxiYWNrKHRoaXMsIG1fb3RoZXIubV9maW5hbGl6ZXJDb250ZXh0KTsKKyAg ICAgICAgICAgIGlmICghbV9maWJlckNvdW50KSB7CisgICAgICAgICAgICAgICAgaWYgKG1fb3Ro ZXIubV9maW5hbGl6ZXJDYWxsYmFjaykKKyAgICAgICAgICAgICAgICAgICAgbV9vdGhlci5tX2Zp bmFsaXplckNhbGxiYWNrKHRoaXMsIG1fb3RoZXIubV9maW5hbGl6ZXJDb250ZXh0KTsKKyAgICAg ICAgICAgIH0gZWxzZSB7CisgICAgICAgICAgICAgICAgdW5zaWduZWQgaSA9IDA7CisgICAgICAg ICAgICAgICAgZG8geworICAgICAgICAgICAgICAgICAgICBSb3BlSW1wbDo6ZGVyZWYobV9vdGhl ci5tX2ZpYmVyc1tpXSk7CisgICAgICAgICAgICAgICAgfSB3aGlsZSAoKytpIDwgbV9maWJlckNv dW50KTsKKyAgICAgICAgICAgIH0KICAgICAgICAgfQogCiAgICAgICAgIGNvbnN0IFVTdHJpbmcm IHZhbHVlKEV4ZWNTdGF0ZSogZXhlYykgY29uc3QKLS0gCjEuNy4zLjQKCg==