"return callback(...);" vs "callback(...); return;"

[Ken]

Assuming that the callback doesn't return a value, does v8 behave any differently when invoking callbacks in one of these forms vs. the other?  I find the first approach to be a convenient shorthand in many cases, but am wondering (after observing some unexpected timings when profiling async methods) if it leads to v8 doing something odd with the stack.

Approach A, return the invoked callback:

function foo(a, callback) {
  var bar = ...;
  return callback(bar);
}

foo("derp", function(b) { ...; return; });

Approach B, invoke callback, then return:

function foo(a, callback) {
   var bar = ...;
   callback(bar);
   return;
}

foo("derp", function(b) { ...; return; });

[Mark Hahn]

The only difference is that the return value of foo takes on the callback's return value instead of null.  There is no other difference at all. 

--
Job Board: http://jobs.nodejs.org/
Posting guidelines: https://github.com/joyent/node/wiki/Mailing-List-Posting-Guidelines
You received this message because you are subscribed to the Google
Groups "nodejs" group.
To post to this group, send email to nod...@googlegroups.com
To unsubscribe from this group, send email to
nodejs+un...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/nodejs?hl=en?hl=en

[Mark Hahn]

BTW, the return in case B is ignored and not needed.

[Joshua Holbrook]

> BTW, the return in case B is ignored and not needed.

What about short circuiting? The example didn't illustrate
short-circuiting but it's somewhat common and probably the OP's
inspiration.

function (opts, cb) {
thing(opts, function (err, data) {
if (err) {
cb(err);
return;
}
var result = transform(data);

cb(null, result);
});
});

In this case, we use return to stop callback execution, and for async
use the actual return value doesn't matter because it can't get
assigned to anything so combining the lines into `return cb(err)`makes
for a reasonable shorthand. Other than that, they're roughly
equivalent.

--Josh

--
Joshua Holbrook
Engineer
Nodejitsu Inc.
jo...@nodejitsu.com

[Roly Fentanes]

I do it during the err check because it's more readable. And I assume everyone else is doing it for the same reason?

function somethingAsync(data, callback) {

  otherthigAsync(function(err, result) {

    if (err) return callback(err);

    // do something with result

  });

});

[Mark Volkmann]

Initially I really hated approach A, but it seems that approach is
very common and it has grown on me. I always use approach A now.

--
R. Mark Volkmann
Object Computing, Inc.

[Tim Caswell]

I also initially preferred approach B "{callback();return}", but now use approach A "return callback()".  Yes there is a slight difference  under the hood, at least in the initial abstract syntax tree.  Usually this is done in an async callback where the return value is going to be ignored anyway.

If any of the V8 developers could shed more light on how these two options affect the optimizer or runtime performance that would be interesting.

I prefer the single statement because:

 - It's a habit from languages with tail recursion.  If you return statement is a call to a function, it's more like a goto in cost than a function call.  JavaScript doesn't have this in any engine I'm aware of so it's more a habit than anything.  (also the return value is almost always ignored or we wouldn't be having this discussion in the first place)

- It's less code.  When in doubt go for less.

[Joshua Cohen]

My preference for option A is because it leaves less room for errors
to creep in. If the invocation of a callback should end processing in
a given branch, then it should be coupled directly with a return. If
you do callback(...); return;, there's a chance that later on someone
might stick some logic between the invocation of the callback and the
return statement which could potentially cause unexpected issues. It
also means that once you get into that habit places where you forget
to return with your callback invocation (which can be a very common
cause of errors in node) really stand out.

Maybe this whole debate goes away in 0.8.x when we have Domains though ;).

[Mark Hahn]

>   there's a chance that later on someone might stick some logic between the invocation of the callback and the return statement 

I view that as an argument for B.  I find it easier to add code before the return with case B.  This is similar to the need to allow commas at the end of lists.

You can add lines of code anywhere that break things.  It is your duty to not be stupid.

[Joshua Cohen]

Yes, and it's easy enough for me to not be stupid, but who can say
what the next person maintaining my code might do ;).

At the end of the day there's not a huge difference between options A
and B, so any preference for one or the other can easily be
questioned, I tend to err on the side of trying to prevent stupid
mistakes through convention whenever possible, in my experience it
tends to keep quality stable over the lifespan of the code's existence
and across many maintainers.

[Mark Hahn]

>  At the end of the day there's not a huge difference between options A
and B, 

Of course.  I just like to argue, which is slightly different than being a troll.

[Ken Woodruff]

Tim Caswell <t...@creationix.com> wrote:

Yes there is a slight difference  under the hood, at least in the initial abstract syntax tree.  Usually this is done in an async callback where the return value is going to be ignored anyway.

Is there an easy (or at least not ridiculously hard) way to get node to spit out the AST in human readable form?

I've used the "return callback()" pattern anywhere that terseness and/or clarity indicated (e.g. short circuits/early returns as others have surmised), but what I'm afraid of is that it's actually causing an issue for some async scenarios.  Because the compiler doesn't know what the callback will return, nor if returning that value from the caller will be meaningful, I'm wondering if it keeps the otherwise unneeded method on the stack so it can wait for the callback to return, and then return that value back up the stack.  In contrast when using approach B the compiler can know explicitly both that the callback's return value is ignored by the caller, and that the caller is not going to return a value, so it's safe for it to immediately pop the caller off the stack (ala tail recursion).  I have no idea if v8 actually does anything like this, it just seems possible.

--Ken

[Tim Caswell]

As I mentioned, JavaScript doesn't have tail call elimination because of how the language is designed. 

http://code.google.com/p/v8/issues/detail?id=457

[Mark Hahn]

>  JavaScript doesn't have tail call elimination because of how the language is designed. 

Is it because of the language design or interpreter design?  Is it because it needs a closure for every call?  It would seem to me that this could be overcome in the interpreter design.

[Oliver Leics]

What about option C, not using 'return':

function (opts, cb) {
thing(opts, function (err, data) {
if (err) {
cb(err);

} else {
var result = transform(data);

cb(null, result);
}
});
});

Aside from readability and number of characters to type, is this
option C better or worse than using return?

[Jann Horn]

Well, when you leave readability and so on away, what's left here? I think
that his option is the ugliest one because it requires you to indent the rest
of the code (unlike guard syntax).

[Oliver Leics]

First: Would you be so kind and stop talking in third person about me
when I'm around? Thank you.

Second: I know that this requires to intend the rest of the code.

But what I really want to know is about technical reasons, not style.
I want to know if it makes any difference to the JIT, v8, or whatever
stacks around. Is not using return an improvement, an deterioration or
does it make no difference for performance/code optimization.

So, under the scope of the question of the OP: What about not using return?

[Jann Horn]

On Fri, Apr 13, 2012 at 08:48:21PM +0200, Oliver Leics wrote:
> On Fri, Apr 13, 2012 at 7:26 PM, Jann Horn <jann...@googlemail.com> wrote:
> > On Fri, Apr 13, 2012 at 10:07:38AM +0200, Oliver Leics wrote:
> >> What about option C, not using 'return':
> >>
> >>    function (opts, cb) {
> >>      thing(opts, function (err, data) {
> >>        if (err) {
> >>          cb(err);
> >>        } else {
> >>          var result = transform(data);
> >>
> >>          cb(null, result);
> >>        }
> >>      });
> >>    });
> >>
> >> Aside from readability and number of characters to type, is this
> >> option C better or worse than using return?
> >
> > Well, when you leave readability and so on away, what's left here? I think
> > that his option is the ugliest one because it requires you to indent the rest
> > of the code (unlike guard syntax).
>
> First: Would you be so kind and stop talking in third person about me
> when I'm around? Thank you.

I'm sorry. I think I wanted to write "this", not "his". That probably looked a bit offensive. :(

> Second: I know that this requires to intend the rest of the code.
>
> But what I really want to know is about technical reasons, not style.
> I want to know if it makes any difference to the JIT, v8, or whatever
> stacks around. Is not using return an improvement, an deterioration or
> does it make no difference for performance/code optimization.
>
> So, under the scope of the question of the OP: What about not using return?

If you're concerned that much about speed, you might want to hack in C or so. :D And if
you believe in microbenchmarks: http://jsperf.com/return-vs-no-return

[Oliver Leics]

>> First: Would you be so kind and stop talking in third person about me
>> when I'm around? Thank you.
>
> I'm sorry. I think I wanted to write "this", not "his". That probably looked a bit offensive. :(

:-) And maybe I was just too thin-skinned.

>> Second: I know that this requires to intend the rest of the code.
>>
>> But what I really want to know is about technical reasons, not style.
>> I want to know if it makes any difference to the JIT, v8, or whatever
>> stacks around. Is not using return an improvement, an deterioration or
>> does it make no difference for performance/code optimization.
>>
>> So, under the scope of the question of the OP: What about not using return?
>
> If you're concerned that much about speed, you might want to hack in C or so. :D And if
> you believe in microbenchmarks: http://jsperf.com/return-vs-no-return

I'm not really concerned, just curious.

And if I would believe in the benchmarks you mentioned, than I had to
say that there is no definite answer.

BTW: It comes from python, where they say: Do not return early, use if else.

[Ken Woodruff]

I've updated that test to include a valueless return; which oddly seems to be the least performant of the three (no return, return value, return without value), but the differences are slight (in my extremely limited testing).

--Ken

[Camilo Aguilar]

I updated the jsperf test to reflect a bit better the scenario being discussed: http://jsperf.com/return-vs-no-return/4 

It seems like v8 optimizes better callback(); return; than return callback();

[mscdex]

On Dec 19, 10:11 am, Camilo Aguilar <camilo.agui...@gmail.com> wrote:
> I updated the jsperf test to reflect a bit better the scenario being
> discussed:http://jsperf.com/return-vs-no-return/4
> It seems like v8 optimizes better callback(); return; than return
> callback();

On Chrome 23 doesReturn is fastest followed by doesValuelessReturn and
then noReturn.

[mscdex]

I forgot to add: it's also faster on the node master branch -- they
must use similar versions of v8?

[Isaac Schlueter]

Relevant to this discussion:
http://mrale.ph/blog/2012/12/15/microbenchmarks-fairy-tale.html

Always profile before you optimize! (Flamegraphs are pretty much the
ideal tool for this.) Then optimize the bits where your program is
*actually* spending time. If you're spending less than 10% of your
programs' time in a particular function, then don't even bother.

If your application's performance is impacted by even as much as 0.5%
as a result of whether you return the cb call, or return right after
it, I'll be flabbergasted. If you are accurately *testing* this with
a microbenchmark (and not just doing silly little dances with the
compiler), then you deserve a medal.

[Isaac Schlueter]

On Wed, Dec 19, 2012 at 12:04 PM, Stephen Bartell <snba...@gmail.com> wrote:

> On Wednesday, December 19, 2012 11:03:34 AM UTC-8, Isaac Schlueter wrote:
>>
>> Relevant to this discussion:
>> http://mrale.ph/blog/2012/12/15/microbenchmarks-fairy-tale.html
>
>

> just because of this one link....
> 1. read the fairy tale, checked out its links.
> a) read the Ask a Foolish question link
> b) then read about Self (from the other universe link).
> 2. went to the rest of the mraleph blog.
> 3. liked his shaky diagrams.
> a) cloned his dart repo and made some distasteful shaky diagrams.
> 4. an hour gone, decided to get back to work...
>
> oh and i had a cup of coffee too.
>
> good read though.

The moral of the story: for optimum performance, never read blog posts
about performance :)