fuwaneko / node-protobuf

Google Protocol Buffers wrapper for Node.js [UNMAINTAINED]
181 stars 42 forks source link

Async variants of functions are not actually asynchronous #80

Open webmakersteve opened 8 years ago

webmakersteve commented 8 years ago

This isn't an issue for me, because I don't use the asynchronous versions of parse or serialize, but the methods themselves are deferred, not asynchronous.

Node only runs thinks on separate threads when they are designated to run in the event loop. Generally these are I/O tasks, like file system operations or network operations. By putting the code in process.nextTick we simply defer the code to execute when the event loop gives the CPU space to run code in the v8 thread again, just deferring the synchronous execution.

This seems like something that an implementer would do rather than something that should be baked into the library. Data serialization and parsing is generally a synchronous concern.

If you do want to make this truly asynchronous, the C++ code would need to spin up another thread and return the data when it's done. See Nan::AsyncWorker.

But since parsing and serialization generally takes under 1 ms anyway, there are not many benefits to offloading the parsing to another thread unless you are dealing with a large amount of data and do not want it to block. But in the current implementation it will still block, just on the next process tick.

Just something to think about.

rynz commented 8 years ago

It looks like Nan::AsyncWorker is an abstraction of uv_queue_work(). Definitely a good idea to implement this suggestion.