90844 2012-07-09 18:35:59 -0700 Avoid creating temporary variables when accessing the size or location in a IntRect or FloatRect object 2012-07-24 10:49:14 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED INVALID P2 Normal --- 1 danakj danakj andersca ap enne jamesr piman simon.fraser webkit.review.bot wjmaclean zlieber oldest_to_newest 665388 0 danakj 2012-07-09 18:35:59 -0700 Avoid creating temporary variables when accessing the size or location in a IntRect or FloatRect object 665391 1 151379 danakj 2012-07-09 18:38:26 -0700 Created attachment 151379 Patch 665726 2 151379 ap 2012-07-10 06:43:42 -0700 Comment on attachment 151379 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. 665757 3 danakj 2012-07-10 07:40:39 -0700 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. 665760 4 ap 2012-07-10 07:56:20 -0700 > 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. 665849 5 151379 enne 2012-07-10 10:12:38 -0700 Comment on attachment 151379 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? 665885 6 simon.fraser 2012-07-10 11:03:51 -0700 You could study the disassembly to see what's actually happening. 666069 7 151379 eric 2012-07-10 14:35:54 -0700 Comment on attachment 151379 Patch OK. LayoutRect? 666102 8 151379 eric 2012-07-10 15:24:19 -0700 Comment on attachment 151379 Patch Yes, w/o performance numbers it's impossible to do performance changes. :) 677020 9 danakj 2012-07-24 10:49:14 -0700 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. 151379 2012-07-09 18:38:26 -0700 2012-07-10 15:24:19 -0700 Patch bug-90844-20120709213825.patch text/plain 2838 danakj U3VidmVyc2lvbiBSZXZpc2lvbjogMTIyMTc1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMzA0MDI4ZDc3MGQ2OTIy M2JiZGMyNWNmMDJkNWQ0OGFhMTYxMDUyNy4uYzZmYTExNzZiYTc2YjE4YWMzOGFmMjEzYWY3NWRh ZjI4ZTJhYThmYiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIyIEBACisyMDEyLTA3LTA5ICBEYW5h IEphbnNlbnMgIDxkYW5ha2pAY2hyb21pdW0ub3JnPgorCisgICAgICAgIEF2b2lkIGNyZWF0aW5n IHRlbXBvcmFyeSB2YXJpYWJsZXMgd2hlbiBhY2Nlc3NpbmcgdGhlIHNpemUgb3IgbG9jYXRpb24g aW4gYSBJbnRSZWN0IG9yIEZsb2F0UmVjdCBvYmplY3QKKyAgICAgICAgaHR0cHM6Ly9idWdzLndl YmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTkwODQ0CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9C T0RZIChPT1BTISkuCisKKyAgICAgICAgVGhlIHNpemUoKSBhbmQgbG9jYXRpb24oKSBtZXRob2Rz IGluIHRoZSBJbnRSZWN0IGNsYXNzIHJldHVybiB0aGUgdmFsdWUKKyAgICAgICAgbWVtYmVyIHZh cmlhYmxlIG9iamVjdHMuIEJ5IHJldHVybmluZyBhIGNvbnN0LXJlZmVyZW5jZSB0byB0aGUgbWVt YmVyCisgICAgICAgIHZhcmlhYmxlLCBpdCBjYW4gYXZvaWQgY3JlYXRpbmcgYSB0ZW1wb3Jhcnkg dmFyaWFibGUgd2hlbiBvbmUgaXMgbm90CisgICAgICAgIG5lZWRlZC4gVGhlIHNhbWUgaXMgdHJ1 ZSBmb3IgRmxvYXRSZWN0LgorCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3MvRmxvYXRSZWN0 Lmg6CisgICAgICAgIChXZWJDb3JlOjpGbG9hdFJlY3Q6OmxvY2F0aW9uKToKKyAgICAgICAgKFdl YkNvcmU6OkZsb2F0UmVjdDo6c2l6ZSk6CisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3MvSW50 UmVjdC5oOgorICAgICAgICAoV2ViQ29yZTo6SW50UmVjdDo6bG9jYXRpb24pOgorICAgICAgICAo V2ViQ29yZTo6SW50UmVjdDo6c2l6ZSk6CisKIDIwMTItMDctMDkgIE5vJ2FtIFJvc2VudGhhbCAg PG5vYW0ucm9zZW50aGFsQG5va2lhLmNvbT4KIAogICAgICAgICBTaGFyZWQgY29kZSB0aGF0IGlz IGd1YXJkZWQgd2l0aCBFTkFCTEUoV0VCR0wpIHNob3VsZCBiZSBndWFyZGVkIHdpdGggVVNFKCkK ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL0Zsb2F0UmVjdC5o IGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvRmxvYXRSZWN0LmgKaW5kZXggNzI1 MDEyZmQ3YTg5Mzk5OTMzNDI0MTczNGU5MjFlMDdiMmQzYTAyMC4uNjMwMTFiNWFiN2YzMDk2MjE4 N2ZkMTcxM2JmNjc3YzM3YmQwN2VlNSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZv cm0vZ3JhcGhpY3MvRmxvYXRSZWN0LmgKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3Jh cGhpY3MvRmxvYXRSZWN0LmgKQEAgLTk1LDggKzk1LDggQEAgcHVibGljOgogCiAgICAgc3RhdGlj IEZsb2F0UmVjdCBuYXJyb3dQcmVjaXNpb24oZG91YmxlIHgsIGRvdWJsZSB5LCBkb3VibGUgd2lk dGgsIGRvdWJsZSBoZWlnaHQpOwogCi0gICAgRmxvYXRQb2ludCBsb2NhdGlvbigpIGNvbnN0IHsg cmV0dXJuIG1fbG9jYXRpb247IH0KLSAgICBGbG9hdFNpemUgc2l6ZSgpIGNvbnN0IHsgcmV0dXJu IG1fc2l6ZTsgfQorICAgIGNvbnN0IEZsb2F0UG9pbnQmIGxvY2F0aW9uKCkgY29uc3QgeyByZXR1 cm4gbV9sb2NhdGlvbjsgfQorICAgIGNvbnN0IEZsb2F0U2l6ZSYgc2l6ZSgpIGNvbnN0IHsgcmV0 dXJuIG1fc2l6ZTsgfQogCiAgICAgdm9pZCBzZXRMb2NhdGlvbihjb25zdCBGbG9hdFBvaW50JiBs b2NhdGlvbikgeyBtX2xvY2F0aW9uID0gbG9jYXRpb247IH0KICAgICB2b2lkIHNldFNpemUoY29u c3QgRmxvYXRTaXplJiBzaXplKSB7IG1fc2l6ZSA9IHNpemU7IH0KZGlmZiAtLWdpdCBhL1NvdXJj ZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL0ludFJlY3QuaCBiL1NvdXJjZS9XZWJDb3JlL3Bs YXRmb3JtL2dyYXBoaWNzL0ludFJlY3QuaAppbmRleCA5YTlhN2Y3ZTVjNTkwZTVkZjc0OTkyNDlm OWI1YThjZWNjZDM5MDY3Li5iYjlmYTRjYmQ5ZTFlMmIwZDk5ODkyZDQ4YzZjODI5OTZhYjhjODIz IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9JbnRSZWN0LmgK KysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvSW50UmVjdC5oCkBAIC05MCw4 ICs5MCw4IEBAIHB1YmxpYzoKICAgICBleHBsaWNpdCBJbnRSZWN0KGNvbnN0IEZsb2F0UmVjdCYp OyAvLyBkb24ndCBkbyB0aGlzIGltcGxpY2l0bHkgc2luY2UgaXQncyBsb3NzeQogICAgIGV4cGxp Y2l0IEludFJlY3QoY29uc3QgRnJhY3Rpb25hbExheW91dFJlY3QmKTsgLy8gZG9uJ3QgZG8gdGhp cyBpbXBsaWNpdGx5IHNpbmNlIGl0J3MgbG9zc3kKICAgICAgICAgCi0gICAgSW50UG9pbnQgbG9j YXRpb24oKSBjb25zdCB7IHJldHVybiBtX2xvY2F0aW9uOyB9Ci0gICAgSW50U2l6ZSBzaXplKCkg Y29uc3QgeyByZXR1cm4gbV9zaXplOyB9CisgICAgY29uc3QgSW50UG9pbnQmIGxvY2F0aW9uKCkg Y29uc3QgeyByZXR1cm4gbV9sb2NhdGlvbjsgfQorICAgIGNvbnN0IEludFNpemUmIHNpemUoKSBj b25zdCB7IHJldHVybiBtX3NpemU7IH0KIAogICAgIHZvaWQgc2V0TG9jYXRpb24oY29uc3QgSW50 UG9pbnQmIGxvY2F0aW9uKSB7IG1fbG9jYXRpb24gPSBsb2NhdGlvbjsgfQogICAgIHZvaWQgc2V0 U2l6ZShjb25zdCBJbnRTaXplJiBzaXplKSB7IG1fc2l6ZSA9IHNpemU7IH0K