Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

with-like performance concerns #13

Open
dead-claudia opened this issue Oct 29, 2017 · 15 comments
Open

with-like performance concerns #13

dead-claudia opened this issue Oct 29, 2017 · 15 comments

Comments

@dead-claudia
Copy link

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.

@samuelgoto
Copy link
Owner

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?

@dead-claudia
Copy link
Author

dead-claudia commented Oct 29, 2017

What is app within the server callback's body?

@samuelgoto
Copy link
Owner

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

@dead-claudia
Copy link
Author

I'm referring to this hypothetical scenario:

const app = express()

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

@samuelgoto
Copy link
Owner

samuelgoto commented Oct 30, 2017

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.
    }
}

@dead-claudia
Copy link
Author

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.

@samuelgoto
Copy link
Owner

samuelgoto commented Oct 31, 2017 via email

@dead-claudia
Copy link
Author

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.

@samuelgoto
Copy link
Owner

samuelgoto commented Nov 1, 2017

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.

@dead-claudia
Copy link
Author

dead-claudia commented Nov 1, 2017 via email

@samuelgoto
Copy link
Owner

samuelgoto commented Nov 2, 2017

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
Copy link
Owner

samuelgoto commented Nov 2, 2017

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")
    }
  }
}

@dead-claudia
Copy link
Author

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
Copy link
Author

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"),
  }
}

@samuelgoto
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants