-
Notifications
You must be signed in to change notification settings - Fork 30
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
Bound methods in classes #39
Comments
Curious, why not just do: const zeroPoint = new Point(0, 0);
const pointString = ::zeroPoint.toString; |
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 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. |
‘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? |
A side-effect of binding a method to an instance is that (1) These two problems are mostly why OO languages tend to only really resolve |
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.
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:
|
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 |
Not sure why I thought the 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. |
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 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. |
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:
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.
@_@ Autobinding is not part of React, isn’t required by React, and did not "make launching React possible". |
@bathos This seems fine to me (it’s not obfuscated): class Foo extends Component {
constructor() {}
@autobind bar() {}
} The |
@simevidas Sure, and as a userland solution this is cool and generally won’t lead to problems. 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...? |
Something like this would be great! The |
@mockdeep binding functions to object can incur significant performance overhead and memory requirements. You generally don't want to bind all functions. |
@Alxandr be careful ... I got yelled at 😅 Though @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 |
@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. |
Interesting, Alxandr, I hadn’t realized that. |
@Alxandr can you give me more specifics on the performance and memory overhead of bound functions? |
@mockdeep effectively you create a new copy of the function for each instance of your class. Also, depending on how you do it ( |
A (perhaps?) interesting sidebar to Alxandr’s explanation: the function returned by |
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. |
That’s true — using 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 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 |
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 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. |
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.
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 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). |
Ah, I see what you're saying about the binding. It looks like this implementation memoizes the result on the first call. |
If you have bound functions, you can't have inheritance. Inheritance only works when you pass the 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 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 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 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 For example, consider a class Button {
constructor(text) {
this.text = text;
}
handleClick(event) {
...
}
} With prototypes, you get this basic memory layout: With a bound 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 ( 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 Wow, this explanation is amazing! Thanks for taking the time to write this out!
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 |
Engines do do stuff like avoiding unnecessary allocations of Regarding the "memoization" — what the implementation you linked to is doing is calling |
Yup :)
They could avoid binding methods that don't have Because objects are first-class, it's not really possible to bind only if some external code needs that method, as we don't know what @bathos I don't know if v8 invalidates the maps when you use the defineProperty primitive to change an existing descriptor. I know it invalidates them when you remove or add properties. I suppose that depends on whether it uses the descriptor flags to do some optimisation or not, and I'm not as knowledgeable on v8's internals :x |
Makes sense. Thanks so much for the education! Really good stuff. |
Just to make my point clear, I'm not saying that binding is bad. I'm saying that having constructs that very easily allows people to autobind things en mass (whole classes) is likely to lead to performance issues. IMHO binding needs to be explicit, because it has a cost. |
I guess I'm still in the boat that this would be a good thing to have. Unless there are clear metrics that show the performance issues are beyond any reasonable use case, I think higher level constructs that abstract away common annoyances in the language should be available. I'm seeing a lot of code that uses arrow functions for everything anyway, without any consideration for the performance implications, so it seems like creating a class level syntax might actually improve VM ability to optimize around these use cases. |
@mockdeep totally — and there will be. It will look like either like the current transform commonly used with React:
or more likely will use a keyword mirroring
Either way these remove the indirection and extra runtime operations of declaring a function as a method only to replace it later with a new bound function instead of just saying what you meant up front: lexically scoped function. Relevant proposals here and here. |
@bathos I think that makes sense, though I don't love the arrow syntax in that. It seems like something like this would be nice: class Foo {
own bar() {
return this.baz;
}
} I'm kind of wondering if it changes much one way or another, though. |
The example you gave would be valid — it has a different meaning, though. That’s method syntax, and you don’t want a method. It would still create a per-instance method, but it would not be lexically scoped. In JS, the syntax for lexically scoped functions is
It does change it, at least as presented here — in the former case we’re still defining a method and converting it to a bound function. In the latter, assuming you use an arrow function, you don’t create the extra function object only to never use it. |
I'm not sure I understand the distinction you're trying to make between what is and is not a method. In this case, I guess what I'm looking for is a method that is lexically scoped. I don't care actually care whether it is class Foo {
bar() => {
return this.baz;
}
} |
That’s a paradox I’m afraid. How could it be lexically scoped while simultaneously being a variable? |
I'm not sure I understand the question. What I'm suggesting is that it is just lexically scoped. |
That means the function needs to close over a specific |
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 ,
The text was updated successfully, but these errors were encountered: