groovy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Graeme Rocher <graeme.roc...@gmail.com>
Subject Re: Proposal: Statically compileable builders
Date Wed, 05 Oct 2016 06:13:40 GMT
Hi Jochen,

Thanks for the interesting analysis of dynamic dispatch via builders
in dynamic Groovy! I feel though the conversation is getting slightly
derailed, this is not about dynamic Groovy this about statically
compiled Groovy. Let us revisit the goals:

1) Allow builders to be used when the class is @CompileStatic without
having to have a separate method marked with @CompileDynamic
2) Allow type checking for certain kinds of builders
3) Improve performance of builders

Most of the discussion has been centered around 3) but IMO 1) and 2)
are equally important. Sure you could make some improvements to the
dynamic side get "almost as good" performance, but that requires the
developer to have advanced knowledge of Groovy's method dispatch and
you won't be able to achieve 1) or 2).

Take for example this code:

void renderJson() {
      def builder = new StreamingJsonBuilder(out)
      Integer num = 10

      builder.message {
            total nun // variable name incorrect here
      }
}

With the current situation there is no way to implement this method
with @CompileStatic on at the class level so you have to mark it with
@CompileDynamic

@CompileDynamic
void renderJson() {
       ...
}

Ok so you have no sacrificed some performance and also lost type
safety. So maybe it is better to isolate just the method that uses the
builder:

void renderJson() {
      def builder = new StreamingJsonBuilder(out)
      Integer num = 10

      renderJsonDynamic(builder, num)
}

@CompileDynamic
void renderJsonDynamic(StreamingJsonBuilder builder, Integer num) {
    builder.message {
            total nun // variable name incorrect here
      }
}

So now you are having to design your code around what parts are
dynamic and what parts are statically compiled which is a code smell.
But worse, this implementation will fail with a
MissingPropertyException for "nun" at runtime, instead of at compile
time.

My proposal differs from Scala's Dynamic in that it should be possible
to selectively implement either "methodMissing" and/or
"propertyMissing". As an example StreamingJsonBuilder would implement
"methodMissing" but not "propertyMissing" this means that you get type
checking on properties, but not on method calls which are dispatched
to the builder.

We could make it smart so that "methodMissing" accepted types and
allowed return values:

StreamingJsonDelegate methodMissing(String name, Closure callable) {
      // ...
}

This would allow you to optimize the static compilation even further
and avoid having to wrap and unwrap in Object[] for when static
compilation is used.

So again let's revisit the benefits of this change:


1) You can write classes annotated with @CompileStatic and use
builders without having to compromise the design of your code
2) You can get type checking for non-builder methods if concrete types
are used in methodMissing (example "methodMissing(String, Closure)"
and type checking for properties if "propertyMissing" is not defined
3) You will get better performance and no matter what we do to the
dynamic side that will stay the same
4) Our goal should be that as much code as possible is compileable
with @CompileStatic and this change helps that


Cheers,



On Tue, Oct 4, 2016 at 10:35 PM, Jochen Theodorou <blackdrag@gmx.org> wrote:
> On 04.10.2016 18:00, Graeme Rocher wrote:
>>
>> [...]  If you combine this
>> will my other proposal around allowing delegates to on maps you can
>> see that you could implement markup builder for static compilation
>>
>> mkp.html {
>>        body {
>>             div id:"test"
>>        }
>> }
>
>
> ok, let us assume mkp is a @SDyn style object/builder and html, body, as
> well as div are actually realized by methodMissing... Where will you then
> put the @DelegatesTo with your Map backed by a static class, that tells us,
> that id is a valid map key for this invocation?
>
>> What you gain is performance, statically compiled builders will be
>> much faster because Groovy throws exceptions during method dispatch
>> (to resolve closure properties). Statically compiled code will give
>> you direct method dispatch to the method, whilst dynamic code will go
>> directly to invokeMethod.
>
>
> But if you want maximum performance you would need to have each element
> backed by a method.
>
>> We see large performance gains when using
>> statically compiled JSON views due to:
>>
>>
>> https://github.com/grails/grails-views/blob/master/core/src/main/groovy/grails/views/compiler/BuilderTypeCheckingExtension.groovy
>
>
> ok, let us put some numbers behind this... Let us look at 3 variants for the
> builders. One is the traditional Groovy style with delegate and
> invokeMethod/methodMissing, the next variant is using mkp.body and mkp.div,
> so a lookup-path using the delegate is not taken. And then of course the
> variant where we actually do not go to methodMissing, but to real methods.
>
> so I made myself a small unprofessional program to measure:
>>
>> 10.times {
>>     def start = System.nanoTime()
>>     1_000_000.times {
>>         foo()
>>     }
>>     def end = System.nanoTime()
>>     println "time : ${(end-start).intdiv(1_000_000)}ms"
>> }
>
>
> to take the most stable value of 10 iterations of 1 million calls. Of course
> only for a small builder. So let us first look at the static cases... what
> @CompileStatic with @DelegatesTo does is actually not depend on the delegate
> of the Closure, but to call the methods directly there... more or less. You
> actually have to get the delegate first, then make the call on the
> delegate... but we will skip this at first. Also I have to mention, that the
> measurement itself costs too, but I ignore this here totally.
>
>> @CompileStatic
>> class Builder {
>>     def methodMissing(String name, args) {
>>         def realArgs = (Object[]) args
>>         def last = realArgs[-1]
>>         if (last instanceof Closure) last.call(name)
>>     }
>> }
>> @CompileStatic
>> def foo() {
>>     def b = new Builder()
>>     b.methodMissing("html") {
>>         b.methodMissing("body") {
>>             b.methodMissing("div", [id:"aDiv"])
>>         }
>>     }
>> }
>
>
> This is about what I think @CompileStatic + @SDyn would produce. Time: ~690
> ms
>
>> @CompileStatic
>> class Builder {
>>     def html(@DelegatesTo(Builder) Closure c) {
>>         c.call()
>>     }
>>     def body(@DelegatesTo(Builder) Closure c) {
>>         c.call()
>>     }
>>     def div(Map m) {
>>     }
>> }
>> @CompileStatic
>> def foo() {
>>     def b = new Builder()
>>     b.html {
>>         b.body {
>>             b.div ([id:"aDiv"])
>>         }
>>     }
>> }
>
>
> This would be without backing by methodMissing. Time: ~280 ms
>
> As you can see, even though we do here direct method calls in both cases, do
> not depend on the MOP... considering that call() will still have a partially
> dynamic call to doCall() here, it could make you wonder where these 400ms
> are going... I was thinking of a megamorphic callsite for last.call(name),
> but I am not sure.
>
>> class Builder {
>>     def methodMissing(String name, args) {
>>         def last = args[-1]
>>         if (last instanceof Closure) {
>>             last.delegate = this
>>             last(name)
>>         }
>>     }
>> }
>>
>> def foo() {
>>     def b = new Builder()
>>     b.html() {
>>         body() {
>>             div (id:"aDiv")
>>         }
>>     }
>> }
>
>
> This is more the classic builder style. Time: ~14600ms
>
> Almost 15s is of course quite the number, about factor 20 to the slower
> static variant. But this can actually be improved:
>
>> class Builder {
>>     def html(c) {
>>         c.delegate = this
>>         c()
>>     }
>>     def body(c) {
>>         c.delegate = this
>>         c()
>>     }
>>     def div(Map m) {
>>     }
>> }
>> def foo() {
>>     def b = new Builder()
>>     b.html() {
>>         body() {
>>             div (id:"aDiv")
>>         }
>>     }
>> }
>
>
> Time: ~870ms
> As you can see, just not going through methodMissing can be a huge
> improvement. Of course this is still like factor 4 compared to the fast
> static variant. But maybe what happens if we also do the same technique to
> not to depend on the delegate?
>
>> class Builder {
>>     def methodMissing(String name, args) {
>>         def last = args[-1]
>>         if (last instanceof Closure) last(name)
>>     }
>> }
>> def foo() {
>>     def b = new Builder()
>>     b.html() {
>>         b.body() {
>>             b.div (id:"aDiv")
>>         }
>>     }
>> }
>
>
> Time: ~870ms
> back to the methodMissing variant and no gain in performance, even though we
> excluded the delegate. But this also shows, the MOP doesn´t have to be slow.
> In my slow example I was actually not setting the delegation strategy:
>
>> class Builder {
>>     def methodMissing(String name, args) {
>>         def last = args[-1]
>>         if (last instanceof Closure) {
>>             last.delegate = this
>>             last.resolveStrategy = Closure.DELEGATE_ONLY
>>             last(name)
>>         }
>>     }
>> }
>>
>> def foo() {
>>     def b = new Builder()
>>     b.html() {
>>         body() {
>>             div (id:"aDiv")
>>         }
>>     }
>> }
>
>
> Time: 1500ms
> Suddenly we are factor 10 faster then before. This is because the default
> strategy will cause the meta class to first search through all owners and
> their parents before looking at the delegate. Depending on the nesting
> level, this can be huge. With this our dynamic standard builder is only at
> about factor 2 compared to your suggestion. And of course factor 6 to the
> faster one.
>
> Adopting the strategy with backing the builder by real methods we could
> actually gain a bit performance:
>
>> class Builder {
>>     def html(c) {
>>         c()
>>     }
>>     def body(c) {
>>         c()
>>     }
>>     def div(Map m) {
>>     }
>> }
>> def foo() {
>>     def b = new Builder()
>>     b.html() {
>>         b.body() {
>>             b.div (id:"aDiv")
>>         }
>>     }
>> }
>
>
> Time: ~870ms
> For this version the resolving strategy actually plays no role, because we
> "statically" resolved that already. But I was not actually true to what
> would be done, was I:
>
>> class Builder {
>>     def html(c) {
>>         c.delegate = this
>>         c.resolveStrategy = Closure.DELEGATE_ONLY
>>         c()
>>     }
>>     def body(c) {
>>         c.delegate = this
>>         c.resolveStrategy = Closure.DELEGATE_ONLY
>>         c()
>>     }
>>     def div(Map m) {
>>     }
>> }
>> def foo() {
>>     def b = new Builder()
>>     b.html() {
>>         delegate.body() {
>>             delegate.div (id:"aDiv")
>>         }
>>     }
>> }
>
>
> Time: ~570ms
> This is actually a bit surprising for me and I have no explanation why this
> version is so much faster. Anyway...
>
>> lass Builder {
>>     def html(c) {
>>         c()
>>     }
>>     def body(c) {
>>         c()
>>     }
>>     def div(Map m) {
>>     }
>> }
>> def foo() {
>>     def b = new Builder()
>>     b.html() {
>>         b.body() {
>>             b.div (id:"aDiv")
>>         }
>>     }
>> }
>
>
> Time: ~500ms, depending on delegate: ~540ms
> This turns out being the fastest dynamic variant.
>
> Ok, one last variant... this mail is probably confusing everyone out there
> already:
>
>> class Builder {
>>     def methodMissing(String name, args) {
>>         def last = args[-1]
>>         if (last instanceof Closure) {
>>             last.delegate = this
>>             last(name)
>>         }
>>     }
>> }
>>
>> def foo() {
>>     def b = new Builder()
>>     b.html() {
>>         delegate.body() {
>>             delegate.div (id:"aDiv")
>>         }
>>     }
>> }
>
>
> Time ~960ms
> so fastest possible methodMissing variant is only factor 1.5 to the static
> methodMissing variant now.
>
> So we have to think about our goal here. Do we want a fully dynamic but
> faster builder, then we should think how to get to the last variant in this
> mail here. Do you want to squeeze out whatever is possible? Then we need to
> talk about replacing Closure as well actually.
>
> bye Jochen
>
>



-- 
Graeme Rocher

Mime
View raw message