Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(934)

Issue 122038: Lars spotted that Handle::operator -> and * are not inlined. That apparently... (Closed)

Created:
11 years, 6 months ago by antonm
Modified:
9 years, 4 months ago
CC:
v8-dev
Visibility:
Public.

Description

Lars spotted that Handle::operator -> and * are not inlined. That apparently buys us almost ~10% speedup for some DOM operations. Committed: http://code.google.com/p/v8/source/detail?r=2148

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -14 lines) Patch
M include/v8.h View 2 chunks +6 lines, -14 lines 4 comments Download

Messages

Total messages: 4 (0 generated)
antonm
11 years, 6 months ago (2009-06-11 14:37:47 UTC) #1
Mads Ager (chromium)
LGTM!
11 years, 6 months ago (2009-06-11 14:51:09 UTC) #2
Kasper Lund
LGTM. http://codereview.chromium.org/122038/diff/1/2 File include/v8.h (right): http://codereview.chromium.org/122038/diff/1/2#newcode215 Line 215: T* operator->() const { I would write ...
11 years, 6 months ago (2009-06-11 14:51:30 UTC) #3
antonm
11 years, 6 months ago (2009-06-11 19:40:47 UTC) #4
tnx for review!  submitting.

http://codereview.chromium.org/122038/diff/1/2
File include/v8.h (right):

http://codereview.chromium.org/122038/diff/1/2#newcode215
Line 215: T* operator->() const {
On 2009/06/11 14:51:30, Kasper Lund wrote:
> I would write this on a single line just like the IsEmpty case.

Done.

http://codereview.chromium.org/122038/diff/1/2#newcode219
Line 219: T* operator*() const {
On 2009/06/11 14:51:30, Kasper Lund wrote:
> Ditto.

Done.

Powered by Google App Engine
This is Rietveld 408576698