Received: by 10.224.105.205 with SMTP id u13mr2601274qao.6.1353054280482;
        Fri, 16 Nov 2012 00:24:40 -0800 (PST)
X-BeenThere: v8-dev@googlegroups.com
Received: by 10.49.74.132 with SMTP id t4ls818821qev.17.gmail; Fri, 16 Nov
 2012 00:24:38 -0800 (PST)
Received: by 10.236.151.13 with SMTP id a13mr2705739yhk.2.1353054278210;
        Fri, 16 Nov 2012 00:24:38 -0800 (PST)
Received: by 10.236.151.13 with SMTP id a13mr2705738yhk.2.1353054278192;
        Fri, 16 Nov 2012 00:24:38 -0800 (PST)
Return-Path: <3RPilUBUJAFc16GDB7JB1D23G3K73L-6G5Bz7A....@2uix4h7xygsz66weerlq.apphosting.bounces.google.com>
Received: from mail-gg0-f197.google.com (mail-gg0-f197.google.com [209.85.161.197])
        by gmr-mx.google.com with ESMTPS id j11si36630ank.2.2012.11.16.00.24.37
        (version=TLSv1/SSLv3 cipher=OTHER);
        Fri, 16 Nov 2012 00:24:37 -0800 (PST)
Received-SPF: pass (google.com: domain of 3RPilUBUJAFc16GDB7JB1D23G3K73L-6G5Bz7A....@2uix4h7xygsz66weerlq.apphosting.bounces.google.com designates 209.85.161.197 as permitted sender) client-ip=209.85.161.197;
Authentication-Results: gmr-mx.google.com; spf=pass (google.com: domain of 3RPilUBUJAFc16GDB7JB1D23G3K73L-6G5Bz7A....@2uix4h7xygsz66weerlq.apphosting.bounces.google.com designates 209.85.161.197 as permitted sender) smtp.mail=3RPilUBUJAFc16GDB7JB1D23G3K73L-6G5Bz7A....@2uix4h7xygsz66weerlq.apphosting.bounces.google.com; dkim=pass header...@chromium.org
Received: by mail-gg0-f197.google.com with SMTP id r5so4106106ggn.0
        for <v8-dev@googlegroups.com>; Fri, 16 Nov 2012 00:24:37 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
        d=chromium.org; s=google;
        h=mime-version:reply-to:x-google-appengine-app-id:message-id:date
         :subject:from:to:cc:content-type;
        bh=Dkxg78zakKwKLunnbXrF5mkLAdaAZE1x0ySpAx2ZNh8=;
        b=JSwqdGAIfPHa8l0h3j22cp3HEHYLTzzDMCH84AwUGTyKojqzXvlcRxpPKIYTc1GlDs
         1FvNvQhhOEw6zi6wBpS309logvJ/UHSRoR4X/MXDHbwTBLzRNwX22N+r6j1JVWrPo4Vr
         WKOQGyBv8MrE3zUKUMkBlP7+KxJcOl4Ye1FOA=
MIME-Version: 1.0
Received: by 10.58.170.6 with SMTP id ai6mr1310485vec.35.1353054276967; Fri,
 16 Nov 2012 00:24:36 -0800 (PST)
Reply-To: svenpa...@chromium.org, mstarzin...@chromium.org, 
	v8-dev@googlegroups.com
Message-ID: <e89a8f839e478a5f8404ce987...@google.com>
Date: Fri, 16 Nov 2012 08:24:36 +0000
Subject: Re: Removed a bunch of GetExistingThreadLocal calls by threading the
 Isolate. (issue 11412007)
From: svenpa...@chromium.org
To: mstarzin...@chromium.org
Cc: v8-dev@googlegroups.com
Content-Type: text/plain; charset=ISO-8859-1; format=flowed; delsp=yes

Landing...


https://codereview.chromium.org/11412007/diff/2001/src/factory.cc
File src/factory.cc (right):

https://codereview.chromium.org/11412007/diff/2001/src/factory.cc#newcode1422
src/factory.cc:1422: Handle<Object> instance_template =
Handle<Object>(desc->instance_template(),
On 2012/11/15 21:24:25, Michael Starzinger wrote:
> Use the handle constructor, that's easier to read I think.

> Handle<Object> instance_template(desc->instance_template(),
isolate());

Done. In general I like the proposed direct initialization much more,
too, because it is conceptually simpler: It is similar to a normal
function call with the usual overloading resolution, just for
constructors. The previous code uses copy initialization, which is much
more tricky: Conceptually it tries to create a temporary of the type of
the LHS (possibly involving conversion functions/operators) and uses a
copy constructor to move the values into the LHS. Luckily enough, GCC is
clever enough to avoid the intermediate steps, but why should we make
things more complicated than necessary? :-)

https://codereview.chromium.org/11412007/diff/2001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/11412007/diff/2001/src/hydrogen.cc#newcode3240
src/hydrogen.cc:3240: Handle<Object>
maybe_type_info(unoptimized_code->type_feedback_info(),
On 2012/11/15 21:24:25, Michael Starzinger wrote:
> I know this isn't your change, it must be something pretty recent. But
I really
> don't like this maybe-handle pattern. The following is much easier to
read and
> doesn't need an explicit isolate parameter.

> Handle<TypeFeedbackInfo> type_info(
>      TypeFeedbackInfo::cast(unoptimized_code->type_feedback_info()));

Done.

https://codereview.chromium.org/11412007/diff/2001/src/hydrogen.cc#newcode7119
src/hydrogen.cc:7119: Handle<Object>
maybe_type_info(unoptimized_code->type_feedback_info(),
On 2012/11/15 21:24:25, Michael Starzinger wrote:
> Likewise.

Done.

https://codereview.chromium.org/11412007/diff/2001/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/11412007/diff/2001/src/runtime.cc#newcode3156
src/runtime.cc:3156: Handle<String>
empty_string(isolate->heap()->empty_string());
On 2012/11/15 21:24:25, Michael Starzinger wrote:
> Just use "Handle<String> empty_string =
isolate->factory()->empty_string();"
> here, saves the handle allocation completely.

Done.

https://codereview.chromium.org/11412007/