Bug 74330 – XHR should use m_responseTypeCode internally to be consistent with WebKit coding style

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Jarred Nicholls]

XHR should use m_responseTypeCode internally for quicker access

[Jarred Nicholls]

Created attachment 118842 [details]
Patch

[Alexey Proskuryakov]

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.

[Jarred Nicholls]

(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.

[Jarred Nicholls]

Committed r102665: <http://trac.webkit.org/changeset/102665>

[Jarred Nicholls]

P.S. thanks for the r+ :)

[Alexey Proskuryakov]

> 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.

[Jarred Nicholls]

(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 :)

[Alexey Proskuryakov]

Given how this function is used, that sounds like a compiler bug. You used a release build, correct?

[Jarred Nicholls]

(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.