Though significantly faster than making a closure, .bind is still fairly
expensive, and creates a heap allocation, which adds to heap pressure and
time spent garbage collecting, which (in our, admittedly, not particularly
normal, case) is our biggest performance concern - with a real-time system
sending 15-30 updates a second to each client, a 1-2 second garbage collect
really puts a damper on things, and anything we can do to reduce this is
helpful.
Additionally, .bind creates undebuggable functions, at least in
node-inspector (you can't step into .bind'd functions), although a quick
test on the latest version of Chrome seems like that is no longer the case
in the version of the debugger and V8 Chrome is using, so once we have a
functional webkit-agent debugger and appropriate V8 version, that's not an
issue.
That being said, in our case, compared to the heap pressure caused by
things like lots of individual Buffers being created for every received
networking packet (we use pooled Buffers for sending, but can't with
received), the .bind overhead is pretty small, so I'd generally agree to
just use the language features, and our friends at Google are constantly
making those more efficient for us.
- Jimb Esser
On Fri, Jun 8, 2012 at 1:22 PM, George Stagas <gsta...@gmail.com> wrote:
> No need to change the API, we have .bind() - use the language
> features, don't reinvent them.
>
> 2012/6/8 Tim Caswell <t...@creationix.com>:
> >
> >
> > On Fri, Jun 8, 2012 at 2:10 PM, tjholowaychuk <tjholoway...@gmail.com>
> > wrote:
> >>
> >> what's wrong with .bind() ?
> >>
> >
> > Mainly the overhead. Bind creates a new function every time it's called,
> > and calling the bound function is a bit slower, especially in V8. (Insert
> > statement about performance only mattering if it's significant...)
> >
> > I usually will bind all my methods from the prototype that will be used
> as
> > callbacks to the instance itself inside the constructor. This gives me a
> > ton more "own" properties, but otherwise is fairly elegant.
> >
> >>
> >> On Jun 8, 11:52 am, AJ ONeal <coola...@gmail.com> wrote:
> >> > emitter.on('data', myModule.dataHandler, myModule);
> >> >
> >> > Even if myModule were to subclass EventEmitter, wouldn't I still need
> to
> >> > pass in the `myModule` instance so that I get the correct `this`? I
> >> > don't
> >> > think I understood what you meant by that.
> >> >
> >> > And then these cases as well:
> >> >
> >> > fs.readFile(file, encoding, myModule.fileHandler, myModule);
> >> >
> >> > process.nextTick(myModule.tickHandler, myModule);
> >> >
> >> > The list goes on. Obviously not a small project. Not difficult either,
> >> > just
> >> > tedious.
> >> >
> >> > AJ ONeal
> >> >
> >> >
> >> >
> >> >
> >> >
> >> >
> >> >
> >> > On Fri, Jun 8, 2012 at 12:42 PM, Tim Caswell <t...@creationix.com>
> >> > wrote:
> >> > > Actually event emitters already call in the scope of the emitter, so
> >> > > there
> >> > > is no need for a specific "this" value there. Just subclass
> >> > > EventEmitter
> >> > > and use normal methods.
> >> >
> >> > > On Fri, Jun 8, 2012 at 1:34 PM, AJ ONeal <coola...@gmail.com>
> wrote:
> >> >
> >> > >> If you're going to use `this` then you must have a callback. It
> would
> >> > >> make no sense to have a `this` and nothing to apply it to.
> >> >
> >> > >> You think EventEmitters would feel the overhead of the if?
> >> >
> >> > >> // context is Array, Object, or Function.
> >> > >> // Numbers, Strings, and Booleans need not `apply` (very punny)
> >> > >> if (context) {
> >> > >> fn.call(context, a, b, c);
> >> > >> } else {
> >> > >> fn(a, b, c);
> >> > >> }
> >> >
> >> > >> As far as the guesswork, well, I hear you on that. I've already
> done
> >> > >> my
> >> > >> ranting at UtahJS Conf. Put this as one more in the bucket of
> reasons
> >> > >> that
> >> > >> callbacks-last was not a well-thought out idea....
> >> >
> >> > >> AJ ONeal
> >> >
> >> > >> On Fri, Jun 8, 2012 at 12:27 PM, Tim Caswell <t...@creationix.com>
> >> > >> wrote:
> >> >
> >> > >>> I think it's a useful addition, but it does cause a little
> overhead
> >> > >>> (though it's probably not noticeable compared to the actual work
> the
> >> > >>> async
> >> > >>> function is doing). EventEmitters might feel the pain since they
> >> > >>> are sync.
> >> > >>> I do worry that it makes things harder for our argument guessing
> >> > >>> code that
> >> > >>> assumes the last arg is a callback. Now there will be an optional
> >> > >>> argument
> >> > >>> after the callback that can be anything (including another
> function)
> >> >
> >> > >>> On Fri, Jun 8, 2012 at 1:18 PM, AJ ONeal <coola...@gmail.com>
> wrote:
> >> >
> >> > >>>> Yes, That's what I am suggesting.
> >> >
> >> > >>>> AJ ONeal
> >> >
> >> > >>>> On Fri, Jun 8, 2012 at 12:17 PM, Tim Caswell
> >> > >>>> <t...@creationix.com>wrote:
> >> >
> >> > >>>>> So this proposal is to modify the API of all async functions to
> >> > >>>>> have
> >> > >>>>> an extra thisp argument after the callback argument (like done
> in
> >> > >>>>> Array.prototype.forEach)?
> >> >
> >> > >>>>> On Fri, Jun 8, 2012 at 1:06 PM, AJ ONeal <coola...@gmail.com>
> >> > >>>>> wrote:
> >> >
> >> > >>>>>> I would like to propose that an additional parameter, `context`
> >> > >>>>>> be
> >> > >>>>>> added to core node modules that accept callbacks to give
> >> > >>>>>> this-ness to the
> >> > >>>>>> callback.
> >> >
> >> > >>>>>> The reason being is that I'm trying to eliminate anonymous
> >> > >>>>>> callbacks
> >> > >>>>>> from my code and have generally cleaner, more readable code (as
> >> > >>>>>> well as
> >> > >>>>>> lower memory usage and less garbage collection).
> >> >
> >> > >>>>>> I don't know if this has been discussed before, but I'd like to
> >> > >>>>>> put
> >> > >>>>>> it on the table.
> >> >
> >> > >>>>>> AJ ONeal
> >
> >
>
