Bug 74330 - XHR should use m_responseTypeCode internally to be consistent with WebKit coding style
Summary: XHR should use m_responseTypeCode internally to be consistent with WebKit cod...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jarred Nicholls
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-12 14:08 PST by Jarred Nicholls
Modified: 2011-12-14 09:05 PST (History)
1 user (show)

See Also:


Attachments
Patch (4.05 KB, patch)
2011-12-12 14:11 PST, Jarred Nicholls
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jarred Nicholls 2011-12-12 14:08:56 PST
XHR should use m_responseTypeCode internally for quicker access
Comment 1 Jarred Nicholls 2011-12-12 14:11:10 PST
Created attachment 118842 [details]
Patch
Comment 2 Alexey Proskuryakov 2011-12-12 20:31:44 PST
Comment on attachment 118842 [details]
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.
Comment 3 Jarred Nicholls 2011-12-13 01:56:22 PST
(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.
Comment 4 Jarred Nicholls 2011-12-13 02:15:11 PST
Committed r102665: <http://trac.webkit.org/changeset/102665>
Comment 5 Jarred Nicholls 2011-12-13 02:26:03 PST
P.S. thanks for the r+ :)
Comment 6 Alexey Proskuryakov 2011-12-13 09:02:30 PST
> 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.
Comment 7 Jarred Nicholls 2011-12-14 05:56:22 PST
(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 :)
Comment 8 Alexey Proskuryakov 2011-12-14 08:47:13 PST
Given how this function is used, that sounds like a compiler bug. You used a release build, correct?
Comment 9 Jarred Nicholls 2011-12-14 09:05:21 PST
(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.