proposal-bind-operator icon indicating copy to clipboard operation
proposal-bind-operator copied to clipboard

Bound methods in classes

Open rektide opened this issue 8 years ago • 42 comments

What about furthering this proposal, & allowing for bound methods on classes? React &al could perhaps benefit from this. Instead of having each consumer have to use a bind operator, the class itself could declare members as bound.

Example, based on @rauschma 's http://www.2ality.com/2015/02/es6-classes-final.html ,

class Point {
    constructor(x, y) {
        this.x = x;
        this.y = y;
    }
    ::toString() {
        return '(' + this.x + ', ' + this.y + ')';
     }
}
const zeroPoint = new Point(0, 0);
const pointString = zeroPoint.toString;
/// ...
console.log(pointString()) //=> (0, 0)

rektide avatar Dec 08 '16 16:12 rektide

Curious, why not just do:

const zeroPoint = new Point(0, 0);
const pointString = ::zeroPoint.toString;

zenparsing avatar Dec 08 '16 16:12 zenparsing

Maybe an actual React example is more useful here, e.g. https://facebook.github.io/react/blog/2015/01/27/react-v0.13.0-beta-1.html#autobinding

jzaefferer avatar Dec 08 '16 16:12 jzaefferer

@jzaefferer Sure, but isn't it "safer" to just do the binding at the access site? (So that it works without any autobinding magic and it works for all kinds of objects, regardless of whether they have autobind magic or not).

This works for any old object:

const toStringer = ::randomObject.toString;

and we don't have to know at the access point whether the method is already auto-bound or not.

zenparsing avatar Dec 08 '16 16:12 zenparsing

‘Autobinding’ instance methods is probably better understood as functionality covered by the class ‘fields’ proposal.

I think one could argue that in either form, this is kind of an antipattern ... what’s the point of classes if the methods cannot live on prototypes?

bathos avatar Dec 08 '16 17:12 bathos

A side-effect of binding a method to an instance is that (1) super will not work anymore for some cases — methods are bound, so they always get the wrong this value; and (2) inheritance with prototypes will also not work anymore, for the same reason — methods are bound, so they always get the wrong this value.

These two problems are mostly why OO languages tend to only really resolve this at call time, rather than at declaration/instantiation time.

robotlolita avatar Dec 10 '16 02:12 robotlolita

As per @jzaefferer's React docs citation & http://www.ian-thomas.net/autobinding-react-and-es6-classes/ , I came to realize one can just create a bound member function via fat arrow syntax.

class MyComponent extends React.Component {
   handler = (e) => { /* stuff */ }
   render(){
      return <div onClick={this.handler}></div>
   }
}

I'd thought that using the bind syntax in declaration made lexical sense (here I am! I'm bound to my this!), and filled a missing gap, and I still think it makes a kind of lexical sense but I no longer see the gap. There is definitely a need- I'd push back on @zenparsing's safe-ness argumentation (follows) given how helpful it is when writing component views, but there exists some way to meet the challenge. I'm not 100% happy with the fat arrow syntax but adding the suggested grammar to bind operator would (while I continue to think look good and make sense and be clearer than fat arrow) add unnecessarily. We definitely do need some way to bind other than at access site. But we have it already. Zenparsing's safenss argumentation:

Sure, but isn't it "safer" to just do the binding at the access site? (So that it works without any autobinding magic and it works for all kinds of objects, regardless of whether they have autobind magic or not).

rektide avatar Dec 30 '16 02:12 rektide

For what it’s worth, the decorators proposal is at Stage 2, so once Babel’s transform is updated, it should be possible to use core-decorators.js to write @autobind class Point {… and have all methods bound automagically.

simevidas avatar Jan 03 '17 04:01 simevidas

Not sure why I thought the handler example I offered was valid. I believe I had mis-understood classes. I thought I'd seen that form work but that must have been a fluke of understanding.

I do think class's function definitions would be a valid, sensible place for bind operators to be used, to declare auto-binding for the function. Decorators seem to have finally waken from their long slumber, so at least simevidas's option will be available if the bind operator doesn't end up being extended to classes. Still, I think classes could benefit from this very common use case & need, and that having it built in to classes capabilities makes more sense than everyone importing various libraries to autobind.

rektide avatar Jan 14 '17 05:01 rektide

Autobinding on instantiation is a severe code smell. Decorators make it, and lots of other things, declaratively possible, but I really don’t believe it should be codified into the language in a syntactic form.

(Clarifying what I mean: If you are autobinding whole ‘classes’, you are doing expensive backflips just to use class syntax where classes are not actually appropriate — i.e., if autobinding is necessary, your code is likely crying out to be refactored into a factory function that returns POJOs.)

bathos avatar Jan 14 '17 06:01 bathos

@bathos I'm not cool with your domineering way of telling people how they should code. Autobinding has many valid uses and many ways it can fight other smelly code. It's comes off to me as enormously egotistical that you could pass blanket judgement on a feature that, frankly, made things possible such as launching React.

rektide avatar Apr 14 '17 21:04 rektide

Sorry, rektide. You can code however you like; however, on forums where people are discussing new syntax and language constructs I think it is reasonable for people to state their opinions about whether a suggestion is problematic.

"Lots of people are doing it" isn’t a good argument, imo, for encouraging it to be done more.

React classes do use their prototypes; the lifecycle methods are actual methods for example. You can already "bind" any custom functionality of a component class to a fixed calling context using existing syntax, for example:

class Foo extends Component {
  constructor() {
    this.bar = () => { /*...*/ };
  }
}

This has a few advantages, but number one in my eyes is that it doesn’t obfuscate the fact that this is a new function per instance, not a shared definition.

It's comes off to me as enormously egotistical that you could pass blanket judgement on a feature that, frankly, made things possible such as launching React.

@_@

Autobinding is not part of React, isn’t required by React, and did not "make launching React possible".

bathos avatar Apr 14 '17 21:04 bathos

@bathos This seems fine to me (it’s not obfuscated):

class Foo extends Component {

  constructor() {}

  @autobind bar() {}
}

The ~~array~~ arrow function in constructor pattern is more of a workaround, until decorators are added to the language, IIUC.

simevidas avatar Apr 14 '17 21:04 simevidas

@simevidas Sure, and as a userland solution this is cool and generally won’t lead to problems. (Not sure what you mean by array function though.) derp arrow of course 👍

But what that decorator is doing matters, I think. It’s actually pretty wild! Most implementations will look something like this:

https://github.com/jayphelps/core-decorators.js/blob/master/src/autobind.js

That chunk of clever magic redefines properties of the class dynamically, not just at declaration time, and (less important) it also adds overhead to each function call. These things won’t kill your app performance, but they are sneaky and weird and they’re the kinds of meta-things you only do when you have a really good reason.

Is the aesthetics of repurposing prototype method syntax as "instance method syntax" really such a reason...?

bathos avatar Apr 14 '17 21:04 bathos

Something like this would be great! The handler = (e) => { /* stuff */ } syntax feels a bit like the tail wagging the dog. We're giving up named functions and adding a little syntactic confusion for the sake of having bound functions. I can't think of a case where I have wanted functions not to be bound to their object, so having a shorthand syntax where I end up with named functions bound to my objects would be great. Right now we just end up with a bindAll in all of our constructors instead.

mockdeep avatar Apr 23 '17 22:04 mockdeep

@mockdeep binding functions to object can incur significant performance overhead and memory requirements. You generally don't want to bind all functions.

Alxandr avatar Apr 24 '17 06:04 Alxandr

@Alxandr be careful ... I got yelled at 😅

Though this resolution was always a point of confusion for folks coming from other languages, I think this particular set of problems wasn’t as prominent before the current generation of DOM libs. That might be because the native DOM (as well as jQuery and other libs from that generation) do the sensible thing — they call handlers against the element context. React may have chosen not to do this with its components because that wouldn’t have aligned with promoting functional approaches, a major theme for them; it would be a specifically OO feature in a lib which discourages OO. If that is the reason, it seems it hasn’t played out so well in practice — people love classes 🤷‍♀️ . Angular I suppose skirted around it because its interior expression language is a thing unto itself; named controllers permit member expressions directly in the templates.

@mockdeep re: "We're giving up named functions" — the function should still be named "handler" here, no? Not sure how the babel transform for this handles it, but in regular ES the value of const foo = () => {} is a function whose name property is "foo".

bathos avatar Apr 24 '17 09:04 bathos

@bathos In react they do set the context to the element though afaik, just like the DOM does. However people expect the context to be the component class instance instead.

Alxandr avatar Apr 24 '17 10:04 Alxandr

Interesting, Alxandr, I hadn’t realized that.

bathos avatar Apr 24 '17 13:04 bathos

@Alxandr can you give me more specifics on the performance and memory overhead of bound functions?

mockdeep avatar Apr 24 '17 15:04 mockdeep

@mockdeep effectively you create a new copy of the function for each instance of your class. Also, depending on how you do it (Function.prototype.bind) it also creates indirection which is not helpful. It's normally not too problematic as long as you do it in your constructor and not in your render method (or similar), cause at least you're not copying the method for every time you render (in react/inferno/whatever), but still if you create a 1000 instances of a Ninja class which has 10 methods, that's 10000 functions in memory. Instead of 10.

Alxandr avatar Apr 25 '17 08:04 Alxandr

A (perhaps?) interesting sidebar to Alxandr’s explanation: the function returned by Function.prototype.bind is an exotic object which has a unique record that references the original function & fixed context / args — though afaik this fact isn’t introspectable externally, this.foo.bind(this) is not quite equivalent to (...args) => this.foo(...args) internally. In the past, calling these special objects was oddly slow. This is fortunately no longer true in current browsers, but as Alxandr said, they are nonetheless unique objects.

bathos avatar Apr 25 '17 11:04 bathos

Based on that article, it seems like a couple of years ago it wasn't great, but 12 seconds for 2 million bound function calls actually doesn't sound all that bad to me. For day-to-day use where there might be a few hundred or less calls, the performance cost is on the order of microseconds. And the performance is presumably a lot better now. I'm more inclined to use a syntax that reduces programmer errors and frustration, and then debug performance issues if/when they crop up.

It's pretty common to use arrow functions all over the place anyway, so these places actually seem to be the least of our concerns. In React we do make a point of not passing down fresh bound functions when rendering, but even then it seems like the performance issues are likely to be minimal unless rendering a few hundred or thousands of elements onto the screen. That case also has less to do with the fact that they are bound functions and more to do with the fact that we're making it impossible to short circuit React's rendering with PureComponent when the props haven't changed. It would be the same if we aggregate together a new object of some sort in the render method to pass down.

mockdeep avatar Apr 25 '17 17:04 mockdeep

That’s true — using .bind or arrow functions for this is pretty unlikely to lead to performance issues speed-wise in any case, and would only lead to performance issues memory-wise in unusual cases. And often enough I think v8 and co may be able to optimize it out of the picture more or less, esp. if you’re focusing on small functions without side effects.

What’s more apt to have a measurable impact is using decorators to dynamically convert methods defined as properties of a prototype to methods bound to specific instances at the time of access. I think this is a pretty bad idea, but when perf is not an issue (and it usually isn’t) this maybe comes down largely to your tolerance of magic & the chance of subtle hidden changes to the shapes of things affecting code in unexpected ways later. That’s what I meant when I said "obfuscating" up above: if one wants an instance method, does it really make sense to create it by declaring a prototype method and calling a function to dynamically convert it to something different on other objects later? A decorator can do anything imaginable, but a good faith contract, I think anyway, might say that decorating a PD of a given object (a) modifies the PD of just the target object and (b) does not alter the original PD type (value in this case).

Since it still makes me sad (really) that rektide has such a foul opinion of me based on what I’d said earlier in the thread, which must have been too polemical, I’d like to clarify that I don’t believe arrow functions, the instance property proposal, or .bind() are bad things you shouldn’t use. What’s trouble are autobind decorators (as described above) and entire "classes" where everything is instance bound. In the latter case, it’s kind of silly because it’s indirect and misleads the reader to believe it’s a "class" when it’s not. If you autobind everything, you’re not using the prototype, which leaves you with just a constructor playing the role of factory function. So it’s a say-what-you-mean thing imo — if all you want is a factory, why add the complexity & extra objects & prototype chain lookups of a construct you specifically don’t want to use? I guess the answer may just be "it looks nicer", and well ... aight.

In the long run, the pending proposal for instance properties which is often used alongside React is likely to become part of the language. It’s basically just sugar that moves this assignments out of the constructor and into the top level class body, though the specific syntax may change before it becomes part of the language. I think once that happens the rest of this is moot, which is why I said this in response to the original proposal in this thread.

bathos avatar Apr 25 '17 18:04 bathos

Makes sense. When you say "at access time" are you talking about when an object is instantiated or when the function is called? I'm all for the decorator based approach or something else that handles binding automatically when the object is instantiated. 99.5 times out of a hundred I want the thing to be bound and am jarred when it's not and I need to do extra work to handle the binding, so I would rather that becomes the implicit case and cases where it's not bound stand out in some way, as there must be a specific reason I would not want to bind something (perhaps with the likes of static).

The whole argument about whether it's actually a class or not seems a little moot to me. Prototypes don't strike me as inherently in conflict with the concept of classes. It's just that instead of having a first class "class" like in some other languages, your class is actually just another object. I've found code written this way less confusing myself and more intention revealing, and it seems like other programmers manage it better as well. Issues with binding per instance may be things that can more effectively be handled under the hood by the engines to the point where it doesn't really matter all that much to us as application developers. Worst case we can still dig down and futz around with prototypes when necessary.

mockdeep avatar Apr 25 '17 19:04 mockdeep

When you say "at access time" are you talking about when an object is instantiated or when the function is called?

Neither. An autobind decorator implementation at the method level must do it at a time of property access, which may or may not be in a context where the value is then invoked as part of the same expression; likely not though, or else you wouldn’t have needed the autobinding in the first place. If it is a class decorator, other options open up that would let you do it at instantiation, but this has caveats (might need to use a new hidden subclass; instance-bound behavior won’t be available till after original cstr runs; ...). There may be other things you could do to polish that case that I’m not thinking of though, not sure, but they’d be weird and very snek.

Prototypes don't strike me as inherently in conflict with the concept of classes.

Agreed — didn’t mean to suggest otherwise. When I said "not a class" I meant specifically in the JS sense of class: a constructable object with a prototype property, along with the things that contract implies. A fully autobound class is still a class technically; I mean it isn’t acting like one, because its normal behaviors are either being suppressed or going unused. That is, for the same reason I wouldn’t write const add = function * (a, b) { return a + b; };/add(1, 2).next().value;, I would not write class Foo { constructor() { this.baz = 3; } @autobind bar() { return this.baz * 7; } } / const foo = new Foo(); const getSevenBazzes = foo.bar;. (The former is a generator — technically — but I didn’t use it as a generator meaningfully. It added overhead for no purpose, and communicated the wrong thing about what it does.)

I think that illustrates it pretty well, but it’s true that some folks would want to use class syntax anyway for reasons "outside" the code itself (visual organization etc).

bathos avatar Apr 25 '17 19:04 bathos

Ah, I see what you're saying about the binding. It looks like this implementation memoizes the result on the first call.

mockdeep avatar Apr 25 '17 20:04 mockdeep

@mockdeep

I can't think of a case where I have wanted functions not to be bound to their object

If you have bound functions, you can't have inheritance. Inheritance only works when you pass the this parameter at the time you're calling a method. It has to be late bound. You can only have one or the other.

You might not care about inheritance. And that's entirely fine. But that's still a massive change in semantics from how JS (and pretty much every other OO language) currently works.

Also, if you have bound functions, you need one separate copy of the function for each instance. JavaScript VMs haven't been designed for this kind of use case, so they can't optimise it. You could share compiled method internals between instances and only add a boundObject field or something, but I'm not sure if that would interfere with current heuristics for type specialisation in the optimising compilers (technically, if the objects have the same Map, it wouldn't need to do branch splitting, but binding methods might give them different Maps if they're doing more advanced type optimisations).

Anyway, my point is: bound methods require more memory and CPU instantiation time (you have to allocate a new copy for all methods when you instantiate an object). They also disallow inheritance, unless you copy inherited methods and rebind them (but that won't go well with Self's map-based optimisation techniques).

robotlolita avatar Apr 25 '17 21:04 robotlolita

@robotlolita inheritance makes sense as a case for not binding, or doing it at runtime. Do you know how much memory the binding costs? So far I'm not aware of it being a cost worth considering as an application developer, and it really seems like the sort of thing the language and engines should be abstracting away from day to day application development.

mockdeep avatar Apr 25 '17 22:04 mockdeep

@mockdeep that really depends on the implementation.

The TL;DR is that you need at the very least N×M times more memory (where N is the number of instances of a particular class, and M the number of bound methods that would otherwise be in the prototype). You could reduce this somewhat by sharing some internals. In the worst case, this could lead to "shape/map/type" explosion: each instance gets its own type, and thus forces everything that they touch either be deoptimised or add a new optimised version for it, so it could be as bad as N²×M×S (where S is each function that uses the object somehow).


For example, consider a Button class:

class Button {
  constructor(text) {
    this.text = text;
  }

  handleClick(event) {
    ...
  }
}

With prototypes, you get this basic memory layout:

screenshot from 2017-04-25 20-48-14

With a bound handleClick, you'd get this memory layout:

screenshot from 2017-04-25 20-49-43

Assuming you were going to bind these anyway, this isn't too bad. Not by itself, at least. The major problem is that the time it takes to instantiate a class that has bound methods is bigger than the time it takes to instantiate a class that does not have bound methods, since you have to allocate more objects. So, if you had a lot of these methods, and had to instantiate a lot of classes, you could hit some GC and allocator pressure. This is very unlikely to be a problem, but it could happen in some edge case (e.g.: React re-rendering a large tree too often).

If instead of binding everything upfront you bind on access (like e.g.: Python does), then instantiation is fast (few objects to allocate), but some method accesses (a::b) would have a higher cost than regular method calls like a.b(...). You pretty much "pay-as-you-go" instead of paying the entire cost upfront.

The problems start when you consider VM optimisations, though. In order to optimise objects based on prototypes (and avoid going up the prototype chain every time you look up a property, which would be unbearably slow), VMs assign shapes based on which properties an object has and what object they inherit from. Roughly, if two objects are created in the same way they'll end up with the same "shape/type". The compiler then uses this information to specialise the code, inlining function calls, replacing property lookups in a hashtable with direct memory lookups, etc. These optimisations make your code fast, but at the expense of quite a bit of memory.

With shapes, the optimising compiler generates a specialised version of a function for each set of parameters you pass to that function. So if you only pass one type to a function, the compiler will generate one generic version (as fallback), one specialised version for that type, and add a type check so if the arguments match the types it has a specialised version of the function for, it runs the fast code.

If you pass two sets of types to a function, the compiler will generate two specialised versions. Three sets of types, three specialised versions. And so on, and so forth. At some point it kinda gives up and deoptimises the entire function because the type checks at the beginning would get too expensive otherwise and your memory usage would grow a lot.

The impact of binding all methods on instantiation would depend on how the VM tracks these shapes. AFAIK v8 only tracks property names in the maps, so as long as objects have the same property names, they'll get the same shape, and everything will be fine. (for functions, their code/environment also matters, as you have to check if you have the same code/stack for inlining and stuff).

Some VMs (like Graal) also track the types of each property. This allows some pretty neat optimisations in the memory layout and with compiling specialised methods, but it has an even higher cost in memory: if objects have the same properties, but those properties hold different types, you get different shapes. And if you get too many different shapes, you have to allocate memory for each one of those + all of the different specialised versions of each method that uses them.

Of course, VMs could always change how they track these shapes to mitigate/solve this. That's not something they do today since this doesn't exist as a feature yet. In general, though, writing VMs tends to be mostly about finding a balance of how much memory you'll use and how fast you want the code to execute (which is why mobile/embedded devices tend to use non-optimising interpreters/compilers: low memory usage is more valued than being super fast).

You don't have this problem when you bind-on-access because you're just creating a new object, that's not a part of your instance anymore. Though OTOH you pay a higher CPU price for bind-on-access.

robotlolita avatar Apr 26 '17 00:04 robotlolita

@robotlolita Wow, this explanation is amazing! Thanks for taking the time to write this out!

You pretty much "pay-as-you-go" instead of paying the entire cost upfront.

It looks like this approach is kind of similar to what is used in the autobind-decorator I mentioned before. On my reading, it binds the method the first time it is called, which like you said, doesn't incur the memory load if the method is never called, but makes the CPU do a little extra work on the fly at run time. Does that sound correct?

Out of curiosity, do you think it would it be possible for the VM to determine whether a method even needs to be bound and ignore the binding if not? As in, if there's no reference to this, it wouldn't bother with the binding and would instead use the prototype method. I guess maybe they might already be doing this with bound functions if it was possible/worthwhile...

mockdeep avatar Apr 26 '17 03:04 mockdeep