Make node module context-aware to allow usage in worker threads

[S4N0I]

Thanks for building and maintaining this module, works great! :)

I came across an issue when trying to perform index lookups on multiple node worker threads:

Error: Module did not self-register: '/usr/src/app/annoy-node/build/Release/addon.node'. at Object.Module._extensions..node (node:internal/modules/cjs/loader:1187:18) at Module.load (node:internal/modules/cjs/loader:981:32) at Function.Module._load (node:internal/modules/cjs/loader:822:12) at Module.require (node:internal/modules/cjs/loader:1005:19) at require (node:internal/modules/cjs/helpers:102:18) at bindings (/usr/src/app/node_modules/bindings/bindings.js:112:48) at Object.<anonymous> (/usr/src/app/annoy-node/index.js:1:37) at Module._compile (node:internal/modules/cjs/loader:1103:14) at Object.Module._extensions..js (node:internal/modules/cjs/loader:1157:10) at Module.load (node:internal/modules/cjs/loader:981:32) Emitted 'error' event on PoolWorker instance at: at PoolWorker.[kOnErrorMessage] (node:internal/worker:289:10) at PoolWorker.[kOnMessage] (node:internal/worker:300:37) at MessagePort.<anonymous> (node:internal/worker:201:57) at MessagePort.[nodejs.internal.kHybridDispatch] (node:internal/event_target:647:20) at MessagePort.exports.emitMessage (node:internal/per_context/messageport:23:28) { code: 'ERR_DLOPEN_FAILED' }

I found a fix for the issue in this thread: https://github.com/nodejs/node/issues/21783#issuecomment-429637117 Apparently, the module hast to be declared in a context-aware manner to be loaded multiple times in the same process. With the changes in this PR, using the index on multiple worker threads works fine.

[jimkang]

Thank you! This looks really clean. I'm really busy for the next several days, but sometime next week, I'll test this out!

Do you have a worker_thread lookup example, BTW?

[S4N0I]

Sounds good! This commit adds a basic test: https://github.com/S4N0I/annoy-node/commit/fe8f450bbfbd33736975c400c0c6ad8cc12f3fff Not perfect but good enough to try it out.

[jimkang]

Apologies: I'm going to have to look at this sometime next week.

[Benjaminrivard]

Hello everyone,

I just encountered the same issue. I would really love to see this PR approved and merged. If I can be of any help, don't hesitate to ask.

Kindly Ben

[jimkang]

Sorry for letting this slip, everyone.

@Benjaminrivard Thanks for the offer! Are you able to run the tests on this branch? make big-test, details. It will involve downloading a file that's several GB and the test takes an hour IIRC.

[Benjaminrivard]

I am experiencing an error when installing with node 16.

# tests 13
# pass  13

# ok

node tests/worker-thread-test.js
internal/modules/cjs/loader.js:638
    throw err;
    ^

Error: Cannot find module 'worker_threads'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)
    at Function.Module._load (internal/modules/cjs/loader.js:562:25)
    at Module.require (internal/modules/cjs/loader.js:692:17)
    at require (internal/modules/cjs/helpers.js:25:18)
    at Object.<anonymous> (/home/benjamin/projects/benjamin/annoy-node-2/node_modules/node-worker-threads-pool/dist/dynamicPool.js:4:26)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
    at Function.Module._load (internal/modules/cjs/loader.js:585:3)
make: *** [Makefile:11 : test] Erreur 1
npm ERR! Test failed.  See above for more details.

I then switched to node 14 and got the following error while running big-test

node tests/basictests.js very-big-config.js
TAP version 13
# Adding vectors to Annoy
not ok 1 More than one vector was added to the index.
  ---
    operator: ok
    expected: true
    actual:   false
    at: DestroyableTransform.checkAdded (/home/benjamin/projects/benjamin/annoy-node-2/tests/basictests.js:45:7)
    stack: |-
      Error: More than one vector was added to the index.
          at Test.assert [as _assert] (/home/benjamin/projects/benjamin/annoy-node-2/node_modules/tape/lib/test.js:228:54)
          at Test.bound [as _assert] (/home/benjamin/projects/benjamin/annoy-node-2/node_modules/tape/lib/test.js:80:32)
          at Test.assert (/home/benjamin/projects/benjamin/annoy-node-2/node_modules/tape/lib/test.js:347:10)
          at Test.bound [as ok] (/home/benjamin/projects/benjamin/annoy-node-2/node_modules/tape/lib/test.js:80:32)
          at DestroyableTransform.checkAdded (/home/benjamin/projects/benjamin/annoy-node-2/tests/basictests.js:45:7)
          at DestroyableTransform.emit (events.js:412:35)
          at endReadableNT (/home/benjamin/projects/benjamin/annoy-node-2/node_modules/readable-stream/lib/_stream_readable.js:1010:12)
          at processTicksAndRejections (internal/process/task_queues.js:82:21)
  ...
ok 2 The index's total vector count is correct.
ok 3 Saved successfully.
# Using vectors from Annoy
ok 4 Loaded successfully.
ok 5 The loaded index's total vector count is correct: 0
not ok 6 getDistance calculates correct distance between items for king and woman
  ---
    operator: equal
    expected: '3.673551'
    actual:   '0.000000'
    at: Test.usingTest (/home/benjamin/projects/benjamin/annoy-node-2/tests/basictests.js:68:5)
    stack: |-
      Error: getDistance calculates correct distance between items for king and woman
          at Test.assert [as _assert] (/home/benjamin/projects/benjamin/annoy-node-2/node_modules/tape/lib/test.js:228:54)
          at Test.bound [as _assert] (/home/benjamin/projects/benjamin/annoy-node-2/node_modules/tape/lib/test.js:80:32)
          at Test.equal (/home/benjamin/projects/benjamin/annoy-node-2/node_modules/tape/lib/test.js:389:10)
          at Test.bound [as equal] (/home/benjamin/projects/benjamin/annoy-node-2/node_modules/tape/lib/test.js:80:32)
          at Test.usingTest (/home/benjamin/projects/benjamin/annoy-node-2/tests/basictests.js:68:5)
          at Test.bound [as _cb] (/home/benjamin/projects/benjamin/annoy-node-2/node_modules/tape/lib/test.js:80:32)
          at Test.run (/home/benjamin/projects/benjamin/annoy-node-2/node_modules/tape/lib/test.js:96:10)
          at Test.bound [as run] (/home/benjamin/projects/benjamin/annoy-node-2/node_modules/tape/lib/test.js:80:32)
          at Immediate.next [as _onImmediate] (/home/benjamin/projects/benjamin/annoy-node-2/node_modules/tape/lib/results.js:83:19)
          at processImmediate (internal/timers.js:464:21)
  ...
ok 7 Vector has correct number of dimensions.
ok 8 Vector contains all numbers.
ok 9 Vector has correct number of dimensions.
ok 10 Vector contains all numbers.
make: *** [Makefile:16: big-test] Erreur de segmentation (core dump créé)

I'm going to investigate on those issues

[jimkang]

Thanks so much for looking! Looks like the module name changed to node:worker_threads?

[Benjaminrivard]

Thanks so much for looking! Looks like the module name changed to node:worker_threads?

My pleasure !

Yes, seems like. Rebuilding the lockfile with a newer version of npm seems to do trick for the first error, so it should not be a big deal. I'm still looking into the second one.

Edit : I'm a bit dummy, I got baited by the file GoogleNews-vectors-negative300.json being there in the fork, but in fact it was empty 🤦‍♀️ I'm trying again, this time it takes a bit more time

[Benjaminrivard]

Sorry for wolf crying, big-test ended with green lights !

benjamin@DESKTOP-2U3L91D:~/dev/annoy-node$ make big-test
node tests/basictests.js very-big-config.js
TAP version 13
# Adding vectors to Annoy
ok 1 More than one vector was added to the index.
ok 2 The index's total vector count is correct.
ok 3 Saved successfully.
# Using vectors from Annoy
ok 4 Loaded successfully.
ok 5 The loaded index's total vector count is correct: 2999980
ok 6 getDistance calculates correct distance between items for king and woman
ok 7 Vector has correct number of dimensions.
ok 8 Vector contains all numbers.
ok 9 Vector has correct number of dimensions.
ok 10 Vector contains all numbers.
ok 11 nnResult is an object.
ok 12 nnResult has a neighbors array.
ok 13 Correct number of neighbors is returned.
ok 14 Neighbors contains all numbers.
ok 15 nnResult has a distances array.
ok 16 Correct number of distances is returned.
ok 17 Distances contains all numbers.
ok 18 nnResultByItem is an object.
ok 19 nnResultByItem has a neighbors array.
ok 20 Correct number of neighbors is returned.
ok 21 Neighbors contains all numbers.
ok 22 nnResultByItem has a distances array.
ok 23 Correct number of distances is returned.
ok 24 Distances contains all numbers.

1..24
# tests 24
# pass  24

# ok

[jimkang]

Nice! Did you do the run with Node 16?

[Benjaminrivard]

Yes ! Nothing seems to be breaking.

I tried with node 18 aswell, but this time the bindings were faulty

[jimkang]

@Benjaminrivard Greatly appreciated! I've been busy and at one point did not actually have enough space on my computer for the large test files. I'll merge and publish soon.

[jimkang]

Publish as 4.0.0. Again, sorry for the delay, and huge thanks @S4N0I and @Benjaminrivard!

[Benjaminrivard]

Thank you very much for taking the time to maintain the library ! Best