kuno / GeoIP

GeoIP binding for nodejs(>=0.10) and iojs
GNU Lesser General Public License v2.1
414 stars 129 forks source link

Asynchronous sleeping #13

Closed jbt closed 13 years ago

jbt commented 13 years ago

Why do all the asynchronous lookups include a 1 second sleep before executing the callback?

kuno commented 13 years ago

"This callback will be ran inside a thread managed by libeio. The first step is to extract the baton structure, so we have access to our context. Next we sleep for the value of baton->sleep_for, which is safe to do, because we are in a thread, and not blocking all Javascript execution in the main thread. Then we do the work of incrementing the count, which if this wasn't just a simple example you would want to wrap in a Lock/Mutex."

https://www.cloudkick.com/blog/2010/aug/23/writing-nodejs-native-extensions/

jbt commented 13 years ago

In that example, the sleep was used just to demonstrate that the execution is non-blocking. It seems to have no use in this module. It's delaying the callback execution for no reason (as far as I can see). If I'm running this module in a situation where I need to look up thousands of IP records every second, I can't have it waiting for a whole second before executing the callback every time.

kuno commented 13 years ago

let me make it clear. The is my first c++ addon for nodejs. So I just followed that tutorial during the whole process of build this package. Honestly, the async(non-blocking) methods are just for demonstration and very rare situations at this point of time. Because the databases are cached in the memory in most cases, the concept of non-blocking dose not apply to these cases. So you should always use synchronous methods (e.g city.lookupSync ), at least in the normal cases. If this make your confused, this is my fault,I am apologize for that.

--kuno