proposal-block-params icon indicating copy to clipboard operation
proposal-block-params copied to clipboard

`with`-like performance concerns

Open dead-claudia opened this issue 8 years ago • 15 comments

The implicit this binding spurs some with-related performance concerns. The callback body could, in sloppy mode, be desugared almost precisely to this:

with (typeof x === "object" && x != null ? x : Object.create(null)) {
    // body
}

For well-known reasons at this point, engines have severely struggled to optimize with, and I suspect they'll have similar objections to this.


Here's a few potential solutions:

  • If a local binding does not exist, then fall back to the with-like behavior. Engines can continue to optimize existing variable accesses, deferring the scope check to globals vs this (which already requires an unavoidable runtime check)

  • Require some sort of prefix/sigil like either one of these (or something completely different altogether):

    server(app) {
        ::get("/route") { ... }
    }
    
    server(app) {
        @get("/route") { ... }
    }
    

    (Note: the second does not conflict with decorators, since you can't "decorate" blocks.)

  • Drop the implicit this binding in nested blocks altogether and just require users to use this.foo.

dead-claudia avatar Oct 29 '17 06:10 dead-claudia

In the current formulation, every nested block gets called with "this". It is not a with like resolution: lexically, it tells where to look at things. For example:

server (app) {
  get("/route") {
  }
}

gets desugared to:

server (app, function() {
   // "this" here gets introduced because it is a block-param call AND it is nested lexically
  this.get("/route", function() {
  });
})

So, no, I don't expect this to have "with"-like resolution semantics (and performance implications and design considersations that go along with "with"-like semantics).

Does that make sense?

samuelgoto avatar Oct 29 '17 06:10 samuelgoto

What is app within the server callback's body?

dead-claudia avatar Oct 29 '17 07:10 dead-claudia

App is the first argument to the call to server. It is defined earlier.

samuelgoto avatar Oct 29 '17 16:10 samuelgoto

I'm referring to this hypothetical scenario:

const app = express()

server(app) {
    get("/route") {
        app // Is this the Express app or `this.app`?
    }
}

dead-claudia avatar Oct 29 '17 21:10 dead-claudia

In the current formulation, app refers to the const app above.

const app = express()

server(app) {
    get("/route") {
        // This is the express app NOT this.app. Normal identifier
        // resolution/scoping applies.
        app

        // The only change in the current formulation is to pre-pend "this" 
        // to inner block params.
        // e.g.
        // app { ... }
        // Would be equivalent to
        // this.app(function() { ... })
        // so, unless it is structured as a block param call, it is evaluated 
        // with the current semantics.
    }
}

samuelgoto avatar Oct 30 '17 16:10 samuelgoto

Oh okay. What if this.app is not defined? What's the fallback?

Also, this precludes potential use cases with nested DSLs, where you could hypothetically have in the browser a routing library, then a render(elem) { ... } call inside to render your view for that particular route. You could solve this by making the top-level DSL invocation explicit and differentiable from a normal call.

dead-claudia avatar Oct 31 '17 09:10 dead-claudia

If this.app is not defined, it is equivalent to let a = undefined; a();. That is, it throws.

Ah, good point about nesting DSLs. A couple of options:

  1. The nested DSL uses the binding operator: e.g. ::render(elem) {}
  2. The nested DSL somehow registers with the outer DSL. e.g. @component class MyElement extends React.Component { ... } with annotations.

Do you think that would be sufficient?

On Tue, Oct 31, 2017 at 2:42 AM, Isiah Meadows [email protected] wrote:

Oh okay. What if this.app is not defined? What's the fallback?

Also, this precludes potential use cases with nested DSLs, where you could hypothetically have in the browser a routing library, then a render(elem) { ... } call inside to render your view for that particular route. You could solve this by making the top-level DSL invocation explicit and differentiable from a normal call.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/samuelgoto/proposal-block-params/issues/13#issuecomment-340710073, or mute the thread https://github.com/notifications/unsubscribe-auth/AAqV6vyMo6i30jmVy-O0XU3nYlhWkuT9ks5sxut4gaJpZM4QKNBf .

-- f u cn rd ths u cn b a gd prgmr !

samuelgoto avatar Oct 31 '17 15:10 samuelgoto

If this.app is not defined, it is equivalent to let a = undefined; a();.

That is, it throws.

Okay, that works as expected.

Ah, good point about nesting DSLs. A couple of options:

  1. The nested DSL uses the binding operator: e.g. ::render(elem) {}
  2. The nested DSL somehow registers with the outer DSL. e.g. @component class MyElement extends React.Component { ... } with annotations.

I'd strongly prefer the first option (independent of syntax) with nesting DSLs, since it avoids making the outer DSL's responsibility to make sure it gets invoked correctly.

Also, for consistency and reduced confusion, we should require that top-level DSLs also use the same operator nested DSLs use. That way, it's 1. clear you're using a DSL and not just plain JS, and 2. there's only one syntax.

dead-claudia avatar Nov 01 '17 07:11 dead-claudia

This thread just made me realize that my current formulation registering DSLs isn't going to work very well. Specifically, some use cases are not DSLs at all, but are control structures. For example:

foreach ({item, index} in array) {
  unless (index % 2 == 0) {
    item.odd = true;
  }
}

Here, foreach and unless are independently provided functions that take block lambdas, so, they are unaware of themselves.

My previous formulation, was something along the lines of:

a {
  b {
  }
}

Would translate to:

a(function() {
  b.call(this, function() {
    ...
  });
})

Which would enable this case to work as:

foreach (array, function({item, index}) {
  unless.call(this, index % 2 == 0, function() {
    item.odd = true
  })
})

As opposed to:

foreach (array, function({item, index}) {
  // this.unless is defined in the call from foreach, which implies
  // it needs to be aware of unless's implementation which
  // doesn't sound desirable.
  this.unless(index % 2 == 0, function() {
    item.odd = true
  })
})

This would enable foreach and unless to be provided independently while still having access to this.

samuelgoto avatar Nov 01 '17 20:11 samuelgoto

And this is why on my first read, I had with-related concerns. It seemed to imply dynamic DSLs (this.foo(...)) while it stated static resolution (foo.call(this, ...)), seemingly confusing the two.

On Wed, Nov 1, 2017, 16:10 sam goto [email protected] wrote:

This thread just made me realize that my current formulation: registering DSLs isn't going to work very well. Specifically, some use cases are not DSLs at all, but are control structures. For example:

foreach ({item, index} in array) { unless (index % 2 == 0) { item.odd = true; } }

Here, foreach and unless are independently provided functions that take block lambdas, so, they are unaware of themselves.

My previous formulation, was something along the lines of:

a { b { } }

Would translate to:

a(function() { b.call(this, function() { ... }); })

Which would enable this case to work as:

foreach (array, function({item, index}) { unless.call(this, index % 2 == 0, function() { item.odd = true }) })

As opposed to:

foreach (array, function({item, index}) { // this.unless is defined in the call from foreach, which implies // it needs to be aware of unless's implementation which // doesn't sound desirable. this.unless(index % 2 == 0, function() { item.odd = true }) })

This would enable foreach and unless to be provided independently while still having access to this.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/samuelgoto/proposal-block-params/issues/13#issuecomment-341226170, or mute the thread https://github.com/notifications/unsubscribe-auth/AERrBK_z74M0x5LbEIH6QF_d9bd6BZI8ks5syNAfgaJpZM4QKNBf .

dead-claudia avatar Nov 01 '17 21:11 dead-claudia

Ah, I see now what you mean by with-like concerns (clarifying question: are you concerned about performance or with-like identifier resolution confusion?).

This formulation could deal with both cases, but would introduce complexity:

a {
  // ...
  b {
    // ...
  }
  // ...
}

desugaring to:

a(function() {
  // ...
  (b in this ? this.b : b)(function() {
     // ...
  })
  // ...
})

Method resolution takes precedence over global/function resolution.

Note that, unlike with:

  • it only happens when you are resolving identifiers for block lambdas rather than everything else (e.g. doesn't work that way for variables, function calls, etc)
  • the object you are looking at for resolving identifiers is fixed (i.e. this)

How does this sound?

samuelgoto avatar Nov 02 '17 16:11 samuelgoto

FWIW, this approach is consistent with Kotlin: it resolves function identifiers looking at "this" first before looking at the global scope. Here is an example:

// This does not get called, unless we remove the "case" method
// in Select.
fun case(condition: Boolean, block: () -> Unit) {
    println("This does not called!")
    block()
}

fun select(condition: Boolean, block: Select.() -> Unit) {
    var sel = Select();
    sel.block()
}

class Select {
    // If this method wasn't here, it would've used
    // the global function case.
    fun case(condition: Boolean, block: () -> Unit) {
       println("This gets called!")
       block()
    }
}

fun main(args: Array<String>) {
  var expr: Boolean = true;
    
  select (expr) {
    case (true) {
        println("Totally true")
    }
    case (true) {
        println("Totally false")
    }
  }
}

samuelgoto avatar Nov 02 '17 17:11 samuelgoto

This formulation could deal with both cases, but would introduce complexity:

a {
  // ...
  b {
    // ...
  }
  // ...
}

desugaring to:

a(function() {
  // ...
  (b in this ? this.b : b)(function() {
     // ...
  })
  // ...
})

I'm still not a fan, because even though you've restricted it to only the calls, it's still sitting in a with problem for DSL invocations only.

Kotlin has static types and method names are always resolved statically, so it can compile everything down to optimized static calls (if they're not inline fun). In a dynamic language where method names are resolved at runtime, this is flat out impossible to do, and ICs for this would be difficult to optimize.

IMHO, an explicit "this starts a DSL" with nested variants being implicit would be best.

dead-claudia avatar Nov 03 '17 04:11 dead-claudia

And in response to a few another parts:

Ah, I see now what you mean by with-like concerns (clarifying question: are you concerned about performance or with-like identifier resolution confusion?).

Both. With Kotlin, you can always verify which type a method comes from, but with JS, it might not be so clear (especially if this is backed by a proxy, which would be entirely reasonable for a DOM builder DSL). Additionally, methods are always auto-bound in Kotlin, so calling a method is not dissimilar to calling a function (and thus they can get away with it), just you have a bound receiver instead of no receiver. (In reality, Kotlin does not really have a concept of a "receiver", just functions and classes with instance methods.)

  • the object you are looking at for resolving identifiers is fixed (i.e. this)

with's lookup is fixed, too - it's just not a variable you can explicitly access within the block. The only difference here is that you don't filter out object[Symbol.unscopables] members, where with does.

[Kotlin example...]

That's probably better written as this, using inline funs to avoid allocating the closure (also, note that it's block(select), not select.block()):

// This does not get called, unless we remove the "case" method
// in Select.
inline fun case(condition: Boolean, block: () -> Unit) {
    println("This does not called!")
    block()
}

class Select {
    // If this method wasn't here, it would've used
    // the global function case.
    inline fun case(condition: Boolean, block: () -> Unit) {
       println("This gets called!")
       block()
    }
}

inline fun select(condition: Boolean, block: Select.() -> Unit) {
    block(Select())
}

fun main(args: Array<String>) {
  val expr = true

  select (expr) {
    case (true) {
        println("Totally true")
    }
    case (false) {
        println("Totally false")
    }
  }
}

// Normally, you'd use `when` instead:
fun main(args: Array<String>) {
  val expr = true

  when (expr) {
    true => println("Totally true"),
    false => println("Totally false"),
  }
}

dead-claudia avatar Nov 03 '17 04:11 dead-claudia

Originally, I had the following desugaring:

a {
  b {
  }
}

Would be rewritten as

a(function() {
  b.call(this, function() {
  })
})

Which would avoid the with-like identifier resolution challenges while still passing the reference to this to enable nesting.

The drawback, is that it requires you to import every single reference to every single function that takes block-params, which could be awkward. In the context of DOM construction, this would be as painful as import {html, body, div, span, img, a, ....} from "framework.js" for every single element you'd need.

samuelgoto avatar Nov 09 '17 19:11 samuelgoto