74330 2011-12-12 14:08:56 -0800 XHR should use m_responseTypeCode internally to be consistent with WebKit coding style 2011-12-14 09:05:21 -0800 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 jarred jarred ap oldest_to_newest 519447 0 jarred 2011-12-12 14:08:56 -0800 XHR should use m_responseTypeCode internally for quicker access 519449 1 118842 jarred 2011-12-12 14:11:10 -0800 Created attachment 118842 Patch 519838 2 118842 ap 2011-12-12 20:31:44 -0800 Comment on attachment 118842 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=118842&action=review > Source/WebCore/ChangeLog:3 > + XHR should use m_responseTypeCode internally for quicker access Using member variables directly is consistent with preferred WebKit coding style, but this change does't affect performance at all. Please update bug and ChangeLog titles before landing. 519961 3 jarred 2011-12-13 01:56:22 -0800 (In reply to comment #2) > (From update of attachment 118842 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=118842&action=review > > > Source/WebCore/ChangeLog:3 > > + XHR should use m_responseTypeCode internally for quicker access > > Using member variables directly is consistent with preferred WebKit coding style, but this change does't affect performance at all. Please update bug and ChangeLog titles before landing. I will modify the title since the consistency is what really matters - but, calling the same function 3 times in a row instead of accessing the variable directly *is* slower (call stack, etc)...but only marginally in the grand scheme of things. 519967 4 jarred 2011-12-13 02:15:11 -0800 Committed r102665: <http://trac.webkit.org/changeset/102665> 519975 5 jarred 2011-12-13 02:26:03 -0800 P.S. thanks for the r+ :) 520113 6 ap 2011-12-13 09:02:30 -0800 > calling the same function 3 times in a row instead of accessing the variable directly *is* slower This is not correct. The generated code is exactly the same in release builds - the function is implicitly inline due to being defined inside class definition. 520888 7 jarred 2011-12-14 05:56:22 -0800 (In reply to comment #6) > > calling the same function 3 times in a row instead of accessing the variable directly *is* slower > > This is not correct. The generated code is exactly the same in release builds - the function is implicitly inline due to being defined inside class definition. Right, but being implicitly marked as "inline" doesn't mean the optimizer will inline it if it's not worth doing. I disassembled a test case from LLVM and it apparently doesn't always do it. The difference in speed was sub-millisecond. Whoopdie do :) 520957 8 ap 2011-12-14 08:47:13 -0800 Given how this function is used, that sounds like a compiler bug. You used a release build, correct? 520977 9 jarred 2011-12-14 09:05:21 -0800 (In reply to comment #8) > Given how this function is used, that sounds like a compiler bug. You used a release build, correct? Agreed, it does sound like a bug. Yes it was release build with inline hints turned on. I haven't tried gcc or clang w/ the same. 118842 2011-12-12 14:11:10 -0800 2011-12-13 02:17:51 -0800 Patch bug-74330-20111212171108.patch text/plain 4149 jarred U3VidmVyc2lvbiBSZXZpc2lvbjogMTAyNjE2CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYzZmMDdlYWFiY2FhMTcy NDY4NmUwMGVjZjI2YThjNDRkYTcyMTAwMy4uMDJjN2NhMGQ1ZjQ2ZjRhNDRiMTA2ZTg3YTk5YWFk ZDhlNTQ5ZmI1ZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDExLTEyLTEyICBKYXJy ZWQgTmljaG9sbHMgIDxqYXJyZWRAc2VuY2hhLmNvbT4KKworICAgICAgICBYSFIgc2hvdWxkIHVz ZSBtX3Jlc3BvbnNlVHlwZUNvZGUgaW50ZXJuYWxseSBmb3IgcXVpY2tlciBhY2Nlc3MKKyAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTc0MzMwCisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTm8gbmV3IHRlc3RzIG5l ZWRlZCwgbm8gYmVoYXZpb3JhbCBjaGFuZ2VzLgorCisgICAgICAgICogeG1sL1hNTEh0dHBSZXF1 ZXN0LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlhNTEh0dHBSZXF1ZXN0OjpyZXNwb25zZVRleHQp OgorICAgICAgICAoV2ViQ29yZTo6WE1MSHR0cFJlcXVlc3Q6OnJlc3BvbnNlWE1MKToKKyAgICAg ICAgKFdlYkNvcmU6OlhNTEh0dHBSZXF1ZXN0OjpyZXNwb25zZUJsb2IpOgorICAgICAgICAoV2Vi Q29yZTo6WE1MSHR0cFJlcXVlc3Q6OmRpZFJlY2VpdmVEYXRhKToKKyAgICAgICAgKiB4bWwvWE1M SHR0cFJlcXVlc3QuaDoKKyAgICAgICAgKFdlYkNvcmU6OlhNTEh0dHBSZXF1ZXN0Ojphc0Jsb2Ip OgorCiAyMDExLTEyLTEyICBPamFuIFZhZmFpICA8b2phbkBjaHJvbWl1bS5vcmc+CiAKICAgICAg ICAgcjEwMjIzNCBjYXVzZWQgUnVsZURhdGEgdG8gdXNlIDMzIGJpdHMgaW4gaXRzIGJpdG1hc2sK ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3htbC9YTUxIdHRwUmVxdWVzdC5jcHAgYi9Tb3Vy Y2UvV2ViQ29yZS94bWwvWE1MSHR0cFJlcXVlc3QuY3BwCmluZGV4IDA1OTBhNDNkNjU1N2ZiYzUw NWRlZjAwNjBkNTJjYWM5MmQyZGZlYjguLmQyYWM4NGFmNDc1M2RlOWQ2Mzc5MTBhNTgzOGEwY2U0 NzQ4ZjU3Y2EgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3htbC9YTUxIdHRwUmVxdWVzdC5j cHAKKysrIGIvU291cmNlL1dlYkNvcmUveG1sL1hNTEh0dHBSZXF1ZXN0LmNwcApAQCAtMjA2LDcg KzIwNiw3IEBAIFhNTEh0dHBSZXF1ZXN0OjpTdGF0ZSBYTUxIdHRwUmVxdWVzdDo6cmVhZHlTdGF0 ZSgpIGNvbnN0CiAKIFN0cmluZyBYTUxIdHRwUmVxdWVzdDo6cmVzcG9uc2VUZXh0KEV4Y2VwdGlv bkNvZGUmIGVjKQogewotICAgIGlmIChyZXNwb25zZVR5cGVDb2RlKCkgIT0gUmVzcG9uc2VUeXBl RGVmYXVsdCAmJiByZXNwb25zZVR5cGVDb2RlKCkgIT0gUmVzcG9uc2VUeXBlVGV4dCkgeworICAg IGlmIChtX3Jlc3BvbnNlVHlwZUNvZGUgIT0gUmVzcG9uc2VUeXBlRGVmYXVsdCAmJiBtX3Jlc3Bv bnNlVHlwZUNvZGUgIT0gUmVzcG9uc2VUeXBlVGV4dCkgewogICAgICAgICBlYyA9IElOVkFMSURf U1RBVEVfRVJSOwogICAgICAgICByZXR1cm4gIiI7CiAgICAgfQpAQCAtMjE1LDcgKzIxNSw3IEBA IFN0cmluZyBYTUxIdHRwUmVxdWVzdDo6cmVzcG9uc2VUZXh0KEV4Y2VwdGlvbkNvZGUmIGVjKQog CiBEb2N1bWVudCogWE1MSHR0cFJlcXVlc3Q6OnJlc3BvbnNlWE1MKEV4Y2VwdGlvbkNvZGUmIGVj KQogewotICAgIGlmIChyZXNwb25zZVR5cGVDb2RlKCkgIT0gUmVzcG9uc2VUeXBlRGVmYXVsdCAm JiByZXNwb25zZVR5cGVDb2RlKCkgIT0gUmVzcG9uc2VUeXBlVGV4dCAmJiByZXNwb25zZVR5cGVD b2RlKCkgIT0gUmVzcG9uc2VUeXBlRG9jdW1lbnQpIHsKKyAgICBpZiAobV9yZXNwb25zZVR5cGVD b2RlICE9IFJlc3BvbnNlVHlwZURlZmF1bHQgJiYgbV9yZXNwb25zZVR5cGVDb2RlICE9IFJlc3Bv bnNlVHlwZVRleHQgJiYgbV9yZXNwb25zZVR5cGVDb2RlICE9IFJlc3BvbnNlVHlwZURvY3VtZW50 KSB7CiAgICAgICAgIGVjID0gSU5WQUxJRF9TVEFURV9FUlI7CiAgICAgICAgIHJldHVybiAwOwog ICAgIH0KQEAgLTI0NCw3ICsyNDQsNyBAQCBEb2N1bWVudCogWE1MSHR0cFJlcXVlc3Q6OnJlc3Bv bnNlWE1MKEV4Y2VwdGlvbkNvZGUmIGVjKQogI2lmIEVOQUJMRShYSFJfUkVTUE9OU0VfQkxPQikK IEJsb2IqIFhNTEh0dHBSZXF1ZXN0OjpyZXNwb25zZUJsb2IoRXhjZXB0aW9uQ29kZSYgZWMpIGNv bnN0CiB7Ci0gICAgaWYgKHJlc3BvbnNlVHlwZUNvZGUoKSAhPSBSZXNwb25zZVR5cGVCbG9iKSB7 CisgICAgaWYgKG1fcmVzcG9uc2VUeXBlQ29kZSAhPSBSZXNwb25zZVR5cGVCbG9iKSB7CiAgICAg ICAgIGVjID0gSU5WQUxJRF9TVEFURV9FUlI7CiAgICAgICAgIHJldHVybiAwOwogICAgIH0KQEAg LTEwNDQsNyArMTA0NCw3IEBAIHZvaWQgWE1MSHR0cFJlcXVlc3Q6OmRpZFJlY2VpdmVEYXRhKGNv bnN0IGNoYXIqIGRhdGEsIGludCBsZW4pCiAgICAgaWYgKG1fc3RhdGUgPCBIRUFERVJTX1JFQ0VJ VkVEKQogICAgICAgICBjaGFuZ2VTdGF0ZShIRUFERVJTX1JFQ0VJVkVEKTsKIAotICAgIGJvb2wg dXNlRGVjb2RlciA9IHJlc3BvbnNlVHlwZUNvZGUoKSA9PSBSZXNwb25zZVR5cGVEZWZhdWx0IHx8 IHJlc3BvbnNlVHlwZUNvZGUoKSA9PSBSZXNwb25zZVR5cGVUZXh0IHx8IHJlc3BvbnNlVHlwZUNv ZGUoKSA9PSBSZXNwb25zZVR5cGVEb2N1bWVudDsKKyAgICBib29sIHVzZURlY29kZXIgPSBtX3Jl c3BvbnNlVHlwZUNvZGUgPT0gUmVzcG9uc2VUeXBlRGVmYXVsdCB8fCBtX3Jlc3BvbnNlVHlwZUNv ZGUgPT0gUmVzcG9uc2VUeXBlVGV4dCB8fCBtX3Jlc3BvbnNlVHlwZUNvZGUgPT0gUmVzcG9uc2VU eXBlRG9jdW1lbnQ7CiAKICAgICBpZiAodXNlRGVjb2RlciAmJiAhbV9kZWNvZGVyKSB7CiAgICAg ICAgIGlmICghbV9yZXNwb25zZUVuY29kaW5nLmlzRW1wdHkoKSkKQEAgLTEwNjgsNyArMTA2OCw3 IEBAIHZvaWQgWE1MSHR0cFJlcXVlc3Q6OmRpZFJlY2VpdmVEYXRhKGNvbnN0IGNoYXIqIGRhdGEs IGludCBsZW4pCiAKICAgICBpZiAodXNlRGVjb2RlcikKICAgICAgICAgbV9yZXNwb25zZUJ1aWxk ZXIuYXBwZW5kKG1fZGVjb2Rlci0+ZGVjb2RlKGRhdGEsIGxlbikpOwotICAgIGVsc2UgaWYgKHJl c3BvbnNlVHlwZUNvZGUoKSA9PSBSZXNwb25zZVR5cGVBcnJheUJ1ZmZlcikgeworICAgIGVsc2Ug aWYgKG1fcmVzcG9uc2VUeXBlQ29kZSA9PSBSZXNwb25zZVR5cGVBcnJheUJ1ZmZlcikgewogICAg ICAgICAvLyBCdWZmZXIgYmluYXJ5IGRhdGEuCiAgICAgICAgIGlmICghbV9iaW5hcnlSZXNwb25z ZUJ1aWxkZXIpCiAgICAgICAgICAgICBtX2JpbmFyeVJlc3BvbnNlQnVpbGRlciA9IFNoYXJlZEJ1 ZmZlcjo6Y3JlYXRlKCk7CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS94bWwvWE1MSHR0cFJl cXVlc3QuaCBiL1NvdXJjZS9XZWJDb3JlL3htbC9YTUxIdHRwUmVxdWVzdC5oCmluZGV4IDY3MTky NWUzNzBmZDI5MzkxYmJhN2JjMzQzMmM2MTcxZmI1NTgyMDAuLmRkNjU4ZjgzNDljMzNkZTdiZDA4 ZjkwNjc3MmViYTQ5OTMyNjVmNWUgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3htbC9YTUxI dHRwUmVxdWVzdC5oCisrKyBiL1NvdXJjZS9XZWJDb3JlL3htbC9YTUxIdHRwUmVxdWVzdC5oCkBA IC04NCw3ICs4NCw3IEBAIHB1YmxpYzoKICAgICBib29sIHdpdGhDcmVkZW50aWFscygpIGNvbnN0 IHsgcmV0dXJuIG1faW5jbHVkZUNyZWRlbnRpYWxzOyB9CiAgICAgdm9pZCBzZXRXaXRoQ3JlZGVu dGlhbHMoYm9vbCwgRXhjZXB0aW9uQ29kZSYpOwogI2lmIEVOQUJMRShYSFJfUkVTUE9OU0VfQkxP QikKLSAgICBib29sIGFzQmxvYigpIGNvbnN0IHsgcmV0dXJuIHJlc3BvbnNlVHlwZUNvZGUoKSA9 PSBSZXNwb25zZVR5cGVCbG9iOyB9CisgICAgYm9vbCBhc0Jsb2IoKSBjb25zdCB7IHJldHVybiBt X3Jlc3BvbnNlVHlwZUNvZGUgPT0gUmVzcG9uc2VUeXBlQmxvYjsgfQogICAgIHZvaWQgc2V0QXNC bG9iKGJvb2wsIEV4Y2VwdGlvbkNvZGUmKTsKICNlbmRpZgogICAgIHZvaWQgb3Blbihjb25zdCBT dHJpbmcmIG1ldGhvZCwgY29uc3QgS1VSTCYsIEV4Y2VwdGlvbkNvZGUmKTsK