Even before the inliner gets it, a JSO chained expression like
a().b().c() is going to look like c(b(a())) so the inliner can only
inline the first call.

Automatically introducing temporaries would seem a lot harder. You'd
have to teach the compiler that 'this' is effectively final, and
therefore any method returning this, effectively always returns the
same value, regardless of side effects. Then, you'd have to introduce
the idea that the compiler can introduce temporaries (speculatively),
and later use common subexpression elimination to detect multiple
identical ones and hoist them out.

e.g.

c(b(a()))

becomes

var t1, t2, t3;
t1 = c(t2 = b(t3 = a()))

then, recognizing the that t1=t2=t3=builder=a() with copy propagation
t1 = builder
a(t1)
b(t1)
c(t1)

and now they can be inlined.

This just seems hard to me, since introducing temporaries is
speculative and you'd have introduce another pass to remove them in
the cases where they provided no benefit. However, this could end up
leading to infinite loops if you're not careful.

-Ray



On Mon, Jun 15, 2009 at 6:11 PM, Brian Stoler<[email protected]> wrote:
>
> Hi gwtc,
>
> I was playing with using super-source to emulate some existing code
> that uses a builder pattern and make it efficient in GWT as a
> JavaScriptObject. I was hoping that the I could make the Builder just
> compile away, but that doesn't seem to be happening. I started
> debugging in JsInliner but started to get a bit lost. This is using
> svn r5557 on trunk FYI (very recent).
>
> What I am seeing is that if you use a chaining call pattern, the
> inliner doesn't realize some of the optimization it can do.
>
> Example usage code:
>
>    void example() {
>        MyData myData = MyData.Builder.create()
>            .setBar("a")
>            .setFoo("b").build();
>
>        Window.alert(myData.getBar());
>        Window.alert(myData.getFoo());
>    }
>
>
> Output I get (extraneous bits removed, PRETTY mode):
>
>    function init(){
>      var myData;
>      myData = $setFoo($setBar(new Object(), 'a'), 'b');
>      $wnd.alert(myData.bar);
>      $wnd.alert(myData.foo);
>    }
>
>    function $setBar(this$static, s){
>      this$static.bar = s;
>      return this$static;
>    }
>
>    function $setFoo(this$static, s){
>      this$static.foo = s;
>      return this$static;
>    }
>
>
> However, with this similar code:
>
>    void example2() {
>        MyData.Builder builder = MyData.Builder.create();
>        builder.setBar("a");
>        builder.setFoo("b");
>
>        MyData myData = builder.build();
>
>        Window.alert(myData.getBar());
>        Window.alert(myData.getFoo());
>    }
>
>
> The code compiles better (no setters):
>
>      builder = new Object();
>      builder.bar = 'a';
>      builder.foo = 'b';
>      myData = builder;
>      $wnd.alert(myData.bar);
>      $wnd.alert(myData.foo);
>
>
> The biggest issue seemed to be that the inliner didn't introduce a
> local variable for "new Object()", and because of that, it correctly
> concluded it couldn't keep copying that expression around. (I traced
> through it having issues due to "new" having side effects.) Even if I
> manually create a variable for the Builder at the outset, it still
> doesn't help. (FYI, I am using new Object() instead of {} to work
> around http://code.google.com/p/google-web-toolkit/issues/detail?id=3568)
>
> I was going to try to dig some more in to the inliner, but any ideas
> how to improve this?
>
> Will a later pass remove useless variable aliasing? (Seems like not
> from my other example?) If so, I was thinking it might work to have
> the inliner introduce a local all the time when it is trying to inline
> a non-void function. Smarter methods are of course possible, but I
> thought that might be worth a shot.
>
> Any other ideas welcome. MyData class attached below. I tried a
> variety of small tweaks to the MyData class and nothing made a
> difference.
>
> FYI, my eventual goal is:
>
> var myData = {bar:'a',foo:'b'};
>
> But that seems like an orthogonal optimization (and perhaps
> generically useful).
>
> Thanks for your interest, if you read this far,
> -brian
>
> PS: I am open to any changes to the MyData code below that a) preserve
> the JSO-ness and b) leave the external API the same.
>
>  public static interface MyData {
>    String getFoo();
>    String getBar();
>
>    public static final class Builder extends JavaScriptObject
> implements MyData {
>      protected Builder() {}
>
>      public static native Builder create() /*-{
>        return new Object();
>      }-*/;
>
>      public native String getFoo() /*-{
>        return this.foo;
>      }-*/;
>
>      public native String getBar() /*-{
>        return this.bar;
>      }-*/;
>
>      public Builder setFoo(String s) {
>        setFooImpl(s);
>        return this;
>      }
>
>      private native void setFooImpl(String s) /*-{
>        this.foo = s;
>      }-*/;
>
>      public Builder setBar(String s) {
>        setBarImpl(s);
>        return this;
>      }
>
>      private native void setBarImpl(String s) /*-{
>        this.bar = s;
>      }-*/;
>
>      public MyData build() {
>        return this;
>      }
>    }
>  }
>
> >
>

--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---

Reply via email to