RESOLVED INVALID90844
Avoid creating temporary variables when accessing the size or location in a IntRect or FloatRect object
https://bugs.webkit.org/show_bug.cgi?id=90844
Summary Avoid creating temporary variables when accessing the size or location in a I...
Dana Jansens
Reported 2012-07-09 18:35:59 PDT
Avoid creating temporary variables when accessing the size or location in a IntRect or FloatRect object
Attachments
Patch (2.77 KB, patch)
2012-07-09 18:38 PDT, Dana Jansens
no flags
Dana Jansens
Comment 1 2012-07-09 18:38:26 PDT
Alexey Proskuryakov
Comment 2 2012-07-10 06:43:42 PDT
Comment on attachment 151379 [details] Patch Does this patch measurably improve performance? These functions are inline, so I don't expect this change to be an improvement. On the other hand, adding an extra deference might theoretically complicate the code enough to confuse optimizer.
Dana Jansens
Comment 3 2012-07-10 07:40:39 PDT
I expect it would be difficult to measure, but these prevent other methods from also returning references, methods who themselves may not be inline. For example, this isn't possible and you must return an IntSize instead because of these methods. const IntSize& Bar::sizeOfSomething(Foo& foo) { return foo.rect().size(); } Typically things that return non-basic types always return a const reference when possible. I have a hard time believing a const reference would produce worse code than a temporary variable.
Alexey Proskuryakov
Comment 4 2012-07-10 07:56:20 PDT
> I have a hard time believing a const reference would produce worse code than a temporary variable. Remember that a reference is the same as a pointer, so unless an optimizer can eliminate such syntactic improvements, you get dereferencing of a pointer with all associated performance issues.
Adrienne Walker
Comment 5 2012-07-10 10:12:38 PDT
Comment on attachment 151379 [details] Patch Are you sure that return value optimization is not in play here? What does the assembly look like before and after on different compilers?
Simon Fraser (smfr)
Comment 6 2012-07-10 11:03:51 PDT
You could study the disassembly to see what's actually happening.
Eric Seidel (no email)
Comment 7 2012-07-10 14:35:54 PDT
Comment on attachment 151379 [details] Patch OK. LayoutRect?
Eric Seidel (no email)
Comment 8 2012-07-10 15:24:19 PDT
Comment on attachment 151379 [details] Patch Yes, w/o performance numbers it's impossible to do performance changes. :)
Dana Jansens
Comment 9 2012-07-24 10:49:14 PDT
Additional reading suggests this change would be a bad one. http://www.macieira.org/blog/2012/02/the-value-of-passing-by-value/ Going to close this, thanks.
Note You need to log in before you can comment on or make changes to this bug.