Yes, I mean write+callback can be sync in some cases. If the callback should be promised to be async, we can use process.nextTick(cb.bind(this)) in Socket._write(). But I think in userland it's not important whether write callback is sync or async. And in current implement callback is also sync if some error occurs.
On Friday, February 1, 2013 11:50:56 AM UTC+8, Mark Hahn wrote: > > That sounds good. > > > if data size is less than system socket buffer ... write() is indeed > nonblock, > > The old method is also non-blocking. I think you meant to say it is sync > instead of async. > > Some people complain if you don't know whether a callback is going to be > async or sync. But I believe code should be written so it doesn't matter. > > > > On Thu, Jan 31, 2013 at 7:41 PM, darcy <[email protected] <javascript:>>wrote: > >> In current version of node, socket write is always 'async', which creates >> some write contexts, put it into queue, and get it out when write data >> finished, and call the callback. But for socket write, if data size is less >> than system socket buffer(which can be modified by >> /proc/sys/net/core/wmem_max), write() is indeed nonblock, and all async >> work can be saved, than the performance of write+callback will be improve >> significantly. >> >> I've forked and made a patch: >> >> https://github.com/freedaxin/node/commit/dba71315bf229b7815c08188ef88097bfbf43135 >> >> I added a benchmark(benchmark/net_rw.js), and the time consumption >> decreased by about 20%. >> >> It doesn't change the user api, but StreamWrap::WriteBuffer() returns a >> Number if all data has been written. >> >> I have run `make jslint test`, found two problems, and changed the test >> code. >> 1. In /test/simple/test-debugger-repl.js, one of the child processes >> always does not exit, I didn't found the reason, and changed the finish >> cleanup behaviours, and passed the test. But I still don't know what's the >> problem. >> >> 2. In /test/simple/test-tcp-wrap-listen.js, because I changed the return >> value of StreamWrap::WriteBuffer(), so the test should be modified. >> >> It's still not a perfect patch, add such a sync feature to async node is >> somehow incompatible with the code logic. And I've noticed that the stream >> related code is not stable yet, so if this patch is acceptable, more >> imporvements can be made later. >> >> btw: windows version has not been changed yet. >> >> issue link: >> https://github.com/joyent/node/issues/4699 >> >> -- >> -- >> 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 [email protected]<javascript:> >> To unsubscribe from this group, send email to >> [email protected] <javascript:> >> For more options, visit this group at >> http://groups.google.com/group/nodejs?hl=en?hl=en >> >> --- >> You received this message because you are subscribed to the Google Groups >> "nodejs" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected] <javascript:>. >> For more options, visit https://groups.google.com/groups/opt_out. >> >> >> > > -- -- 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 [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/nodejs?hl=en?hl=en --- You received this message because you are subscribed to the Google Groups "nodejs" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/groups/opt_out.
