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

S4N0I · 2022-04-27T11:07:39Z

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: nodejs/node#21783 (comment) 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.

See also: nodejs/node#21783 (comment)

jimkang · 2022-04-28T01:29:39Z

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 · 2022-04-30T16:09:48Z

Sounds good!
This commit adds a basic test: S4N0I@fe8f450
Not perfect but good enough to try it out.

jimkang · 2022-05-06T16:04:02Z

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

Benjaminrivard · 2023-08-08T12:45:08Z

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 · 2023-08-08T14:44:07Z

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 · 2023-08-08T15:20:01Z

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 · 2023-08-08T15:32:51Z

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

Benjaminrivard · 2023-08-08T15:39:16Z

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 · 2023-08-08T16:18:27Z

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 · 2023-08-08T16:28:14Z

Nice! Did you do the run with Node 16?

Benjaminrivard · 2023-08-08T16:31:56Z

Yes ! Nothing seems to be breaking.

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

jimkang · 2023-08-08T16:43:47Z

@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 · 2023-08-08T17:12:12Z

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

Benjaminrivard · 2023-08-08T17:36:32Z

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

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

1681bd6

See also: nodejs/node#21783 (comment)

Add test for usage with worker threads

fe8f450

jimkang merged commit 39eaca2 into jimkang:master Aug 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

S4N0I commented Apr 27, 2022

jimkang commented Apr 28, 2022

S4N0I commented Apr 30, 2022

jimkang commented May 6, 2022

Benjaminrivard commented Aug 8, 2023

jimkang commented Aug 8, 2023

Benjaminrivard commented Aug 8, 2023 •

edited

Loading

jimkang commented Aug 8, 2023

Benjaminrivard commented Aug 8, 2023 •

edited

Loading

Benjaminrivard commented Aug 8, 2023

jimkang commented Aug 8, 2023

Benjaminrivard commented Aug 8, 2023

jimkang commented Aug 8, 2023

jimkang commented Aug 8, 2023

Benjaminrivard commented Aug 8, 2023

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

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

Conversation

S4N0I commented Apr 27, 2022

jimkang commented Apr 28, 2022

S4N0I commented Apr 30, 2022

jimkang commented May 6, 2022

Benjaminrivard commented Aug 8, 2023

jimkang commented Aug 8, 2023

Benjaminrivard commented Aug 8, 2023 • edited Loading

jimkang commented Aug 8, 2023

Benjaminrivard commented Aug 8, 2023 • edited Loading

Benjaminrivard commented Aug 8, 2023

jimkang commented Aug 8, 2023

Benjaminrivard commented Aug 8, 2023

jimkang commented Aug 8, 2023

jimkang commented Aug 8, 2023

Benjaminrivard commented Aug 8, 2023

Benjaminrivard commented Aug 8, 2023 •

edited

Loading

Benjaminrivard commented Aug 8, 2023 •

edited

Loading