Esm off thread unlock worker #2

targos · 2023-01-01T14:19:51Z

I compared the test results before and after the changes (deleted identical output).

Before (16 failures)

=== release test-esm-loader ===
Path: es-module/test-esm-loader
(node:3020513) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
node:internal/process/esm_loader:19
    internalBinding('errors').triggerUncaughtException(
                              ^

RangeError [ERR_OUT_OF_RANGE]: The value of "serializedResponse.byteLength" is out of range. It must be <= 2048. Received 2651
    at new NodeError (node:internal/errors:399:5)
    at setupESMWorker (node:internal/modules/esm/worker:80:15)

Node.js v20.0.0-pre
Command: out/Release/node --experimental-loader ./test/fixtures/es-module-loaders/hooks-custom.mjs /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader.mjs


=== release test-esm-loader-stringify-text ===
Path: es-module/test-esm-loader-stringify-text
(node:3020959) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
node:internal/process/esm_loader:19
    internalBinding('errors').triggerUncaughtException(
                              ^

RangeError [ERR_OUT_OF_RANGE]: The value of "serializedResponse.byteLength" is out of range. It must be <= 2048. Received 2511
    at new NodeError (node:internal/errors:399:5)
    at setupESMWorker (node:internal/modules/esm/worker:80:15)

Node.js v20.0.0-pre
Command: out/Release/node --experimental-loader ./test/fixtures/es-module-loaders/string-sources.mjs /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-stringify-text.mjs


=== release test-esm-named-exports ===
Path: es-module/test-esm-named-exports
(node:3021177) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
node:internal/process/esm_loader:19
    internalBinding('errors').triggerUncaughtException(
                              ^

RangeError [ERR_OUT_OF_RANGE]: The value of "serializedResponse.byteLength" is out of range. It must be <= 2048. Received 5398
    at new NodeError (node:internal/errors:399:5)
    at setupESMWorker (node:internal/modules/esm/worker:80:15)

Node.js v20.0.0-pre
Command: out/Release/node --experimental-loader ./test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs /home/mzasso/git/nodejs/node/test/es-module/test-esm-named-exports.mjs


=== release test-esm-loader-obsolete-hooks ===
Path: es-module/test-esm-loader-obsolete-hooks
TAP version 13
# Subtest: ESM: deprecation warnings for obsolete hooks
    # Subtest: <anonymous>
    not ok 1 - <anonymous>
      ---
      duration_ms: 223.595027
      failureType: 'testCodeFailure'
      error: |-
        The input did not match the regular expression /DeprecationWarning:/. Input:

        'Bad command or file name\n'

      code: 'ERR_ASSERTION'
      actual: |-
        Bad command or file name

      operator: 'match'
      stack: |-
        Object.<anonymous> (file:///home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-obsolete-hooks.mjs:20:5)
        async ItTest.run (node:internal/test_runner/test:548:9)
        async Promise.all (index 0)
        async Suite.run (node:internal/test_runner/test:798:7)
      ...
    1..1
not ok 1 - ESM: deprecation warnings for obsolete hooks
  ---
  duration_ms: 226.322374
  failureType: 'subtestsFailed'
  error: '1 subtest failed'
  code: 'ERR_TEST_FAILURE'
  ...
1..1
# tests 1
# pass 0
# fail 1
# cancelled 0
# skipped 0
# todo 0
# duration_ms 232.123419
Command: out/Release/node /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-obsolete-hooks.mjs


=== release test-esm-loader-chaining ===
Path: es-module/test-esm-loader-chaining
TAP version 13
# Subtest: ESM: loader chaining
    # Subtest: should load unadulterated source when there are no loaders
    ok 1 - should load unadulterated source when there are no loaders
      ---
      duration_ms: 295.432017
      ...
    # Subtest: should load properly different source when only load changes something
    ok 2 - should load properly different source when only load changes something
      ---
      duration_ms: 292.425237
      ...
    # Subtest: should result in proper output from multiple changes in resolve hooks
    ok 3 - should result in proper output from multiple changes in resolve hooks
      ---
      duration_ms: 289.996562
      ...
    # Subtest: should respect modified context within resolve chain
    ok 4 - should respect modified context within resolve chain
      ---
      duration_ms: 286.858653
      ...
    # Subtest: should accept only the correct arguments
    not ok 5 - should accept only the correct arguments
      ---
      duration_ms: 337.407822
      failureType: 'testCodeFailure'
      error: |-
        The input did not match the regular expression /specifier: 'node:fs'/. Input:

        'resolve arg count: 3\n' +
          '{"specifier":"node:fs","context":{"conditions":["node","import","node-addons"],"importAssertions":{},"parentURL":"file:///home/mzasso/git/nodejs/node/[eval1]"}}\n' +
          'load arg count: 3{"url":"node:fs","context":{"importAssertions":{}}}\n' +
          'resolve arg count: 3\n' +
          '{"specifier":"node:fs","context":{"conditions":["node","import","node-addons"],"importAssertions":{},"parentURL":"file:///home/mzasso/git/nodejs/node/[eval1]"}}\n' +
          'load arg count: 3{"url":"node:fs","context":{}}\n'

      code: 'ERR_ASSERTION'
      actual: |-
        resolve arg count: 3
        {"specifier":"node:fs","context":{"conditions":["node","import","node-addons"],"importAssertions":{},"parentURL":"file:///home/mzasso/git/nodejs/node/[eval1]"}}
        load arg count: 3{"url":"node:fs","context":{"importAssertions":{}}}
        resolve arg count: 3
        {"specifier":"node:fs","context":{"conditions":["node","import","node-addons"],"importAssertions":{},"parentURL":"file:///home/mzasso/git/nodejs/node/[eval1]"}}
        load arg count: 3{"url":"node:fs","context":{}}

      operator: 'match'
      stack: |-
        Object.<anonymous> (file:///home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-chaining.mjs:117:12)
        async ItTest.run (node:internal/test_runner/test:548:9)
        async Promise.all (index 4)
        async Suite.run (node:internal/test_runner/test:798:7)
      ...
    # Subtest: should result in proper output from multiple changes in resolve hooks
    ok 6 - should result in proper output from multiple changes in resolve hooks
      ---
      duration_ms: 287.057979
      ...
    # Subtest: should provide the correct "next" fn when multiple calls to next within same loader
    ok 7 - should provide the correct "next" fn when multiple calls to next within same loader
      ---
      duration_ms: 269.567886
      ...
    # Subtest: should use the correct `name` for next<HookName>'s function
    not ok 8 - should use the correct `name` for next<HookName>'s function
      ---
      duration_ms: 345.105218
      failureType: 'testCodeFailure'
      error: |-
        The input did not match the regular expression /next<HookName>: nextResolve/. Input:

        'resolve 42\nnext<HookName>:nextResolve\n42\n'

      code: 'ERR_ASSERTION'
      actual: |-
        resolve 42
        next<HookName>:nextResolve
        42

      operator: 'match'
      stack: |-
        Object.<anonymous> (file:///home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-chaining.mjs:188:12)
        process.processTicksAndRejections (node:internal/process/task_queues:95:5)
        async ItTest.run (node:internal/test_runner/test:548:9)
        async Promise.all (index 7)
        async Suite.run (node:internal/test_runner/test:798:7)
      ...
    # Subtest: should throw for incomplete resolve chain, citing errant loader & hook
    ok 9 - should throw for incomplete resolve chain, citing errant loader & hook
      ---
      duration_ms: 262.020809
      ...
    # Subtest: should NOT throw when nested resolve hook signaled a short circuit
    ok 10 - should NOT throw when nested resolve hook signaled a short circuit
      ---
      duration_ms: 403.642323
      ...
    # Subtest: should NOT throw when nested load hook signaled a short circuit
    ok 11 - should NOT throw when nested load hook signaled a short circuit
      ---
      duration_ms: 355.875815
      ...
    # Subtest: should allow loaders to influence subsequent loader resolutions
    not ok 12 - should allow loaders to influence subsequent loader resolutions
      ---
      duration_ms: 307.344266
      failureType: 'testCodeFailure'
      error: |-
        Expected values to be strictly equal:
        + actual - expected

        + 'node:internal/process/esm_loader:19\n' +
        +   "    internalBinding('errors').triggerUncaughtException(\n" +
        +   '                              ^\n' +
        +   '\n' +
        +   "Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'xxx' imported from /home/mzasso/git/nodejs/node/test/fixtures/es-module-loaders/\n" +
        +   '    at new NodeError (node:internal/errors:399:5)\n' +
        +   '    at packageResolve (node:internal/modules/esm/resolve:795:9)\n' +
        +   '    at moduleResolve (node:internal/modules/esm/resolve:844:20)\n' +
        +   '    at DefaultModuleLoader.defaultResolve (node:internal/modules/esm/resolve:1069:11)\n' +
        +   '    at DefaultModuleLoader.resolve (node:internal/modules/esm/loader:298:32)\n' +
        +   '    at DefaultModuleLoader.getModuleJob (node:internal/modules/esm/loader:150:34)\n' +
        +   '    at DefaultModuleLoader.import (node:internal/modules/esm/loader:252:10)\n' +
        +   '    at initializeHooks (node:internal/modules/esm/utils:122:65)\n' +
        +   '    at async setupESMWorker (node:internal/modules/esm/worker:44:13)\n' +
        +   '\n' +
        +   'Node.js v20.0.0-pre\n'
        - ''
      code: 'ERR_ASSERTION'
      expected: ''
      actual: |-
        node:internal/process/esm_loader:19
            internalBinding('errors').triggerUncaughtException(
                                      ^

        Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'xxx' imported from /home/mzasso/git/nodejs/node/test/fixtures/es-module-loaders/
            at new NodeError (node:internal/errors:399:5)
            at packageResolve (node:internal/modules/esm/resolve:795:9)
            at moduleResolve (node:internal/modules/esm/resolve:844:20)
            at DefaultModuleLoader.defaultResolve (node:internal/modules/esm/resolve:1069:11)
            at DefaultModuleLoader.resolve (node:internal/modules/esm/loader:298:32)
            at DefaultModuleLoader.getModuleJob (node:internal/modules/esm/loader:150:34)
            at DefaultModuleLoader.import (node:internal/modules/esm/loader:252:10)
            at initializeHooks (node:internal/modules/esm/utils:122:65)
            at async setupESMWorker (node:internal/modules/esm/worker:44:13)

        Node.js v20.0.0-pre

      operator: 'strictEqual'
      stack: |-
        Object.<anonymous> (file:///home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-chaining.mjs:268:12)
        process.processTicksAndRejections (node:internal/process/task_queues:95:5)
        async ItTest.run (node:internal/test_runner/test:548:9)
        async Promise.all (index 11)
        async Suite.run (node:internal/test_runner/test:798:7)
      ...
    # Subtest: should throw when the resolve chain is broken
    ok 13 - should throw when the resolve chain is broken
      ---
      duration_ms: 302.525516
      ...
    # Subtest: should throw for incomplete load chain, citing errant loader & hook
    ok 14 - should throw for incomplete load chain, citing errant loader & hook
      ---
      duration_ms: 273.592653
      ...
    # Subtest: should throw when the load chain is broken
    ok 15 - should throw when the load chain is broken
      ---
      duration_ms: 267.83066
      ...
    # Subtest: should throw when invalid `specifier` argument passed to `nextResolve`
    ok 16 - should throw when invalid `specifier` argument passed to `nextResolve`
      ---
      duration_ms: 261.824183
      ...
    # Subtest: should throw when resolve hook is invalid
    ok 17 - should throw when resolve hook is invalid
      ---
      duration_ms: 255.64576
      ...
    # Subtest: should throw when invalid `context` argument passed to `nextResolve`
    ok 18 - should throw when invalid `context` argument passed to `nextResolve`
      ---
      duration_ms: 291.104702
      ...
    # Subtest: should throw when load hook is invalid
    ok 19 - should throw when load hook is invalid
      ---
      duration_ms: 225.781249
      ...
    # Subtest: should throw when invalid `url` argument passed to `nextLoad`
    ok 20 - should throw when invalid `url` argument passed to `nextLoad`
      ---
      duration_ms: 259.113417
      ...
    # Subtest: should throw when invalid `url` argument passed to `nextLoad`
    ok 21 - should throw when invalid `url` argument passed to `nextLoad`
      ---
      duration_ms: 277.272299
      ...
    # Subtest: should throw when invalid `context` argument passed to `nextLoad`
    ok 22 - should throw when invalid `context` argument passed to `nextLoad`
      ---
      duration_ms: 240.81358
      ...
    1..22
not ok 1 - ESM: loader chaining
  ---
  duration_ms: 527.078898
  failureType: 'subtestsFailed'
  error: '3 subtests failed'
  code: 'ERR_TEST_FAILURE'
  ...
1..1
# tests 1
# pass 0
# fail 1
# cancelled 0
# skipped 0
# todo 0
# duration_ms 531.606638
Command: out/Release/node /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-chaining.mjs


[06:17|% 100|+ 3795|-  16]: Done

Failed tests:
out/Release/node --experimental-loader ./test/fixtures/es-module-loaders/assertionless-json-import.mjs /home/mzasso/git/nodejs/node/test/es-module/test-esm-assertionless-json-import.js
out/Release/node --experimental-loader ./test/fixtures/es-module-loaders/example-loader.mjs /home/mzasso/git/nodejs/node/test/es-module/test-esm-example-loader.mjs
out/Release/node --experimental-import-meta-resolve /home/mzasso/git/nodejs/node/test/es-module/test-esm-import-meta-resolve.mjs
out/Release/node --experimental-loader ./test/fixtures/es-module-loaders/hooks-custom.mjs /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader.mjs
out/Release/node /home/mzasso/git/nodejs/node/test/es-module/test-esm-import-flag.mjs
out/Release/node --loader ./test/fixtures/es-module-loaders/mock-loader.mjs /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-mock.mjs
out/Release/node --experimental-loader ./test/fixtures/es-module-loaders/loader-invalid-url.mjs /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-invalid-url.mjs
out/Release/node --experimental-loader ./test/fixtures/es-module-loaders/loader-side-effect.mjs --require ./test/fixtures/es-module-loaders/loader-side-effect-require-preload.js /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-side-effect.mjs
out/Release/node --experimental-loader ./test/fixtures/es-module-loaders/string-sources.mjs /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-stringify-text.mjs
out/Release/node --loader ./test/fixtures/es-module-loaders/hook-resolve-type.mjs /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-resolve-type.mjs
out/Release/node --experimental-loader ./test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs /home/mzasso/git/nodejs/node/test/es-module/test-esm-named-exports.mjs
out/Release/node /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-obsolete-hooks.mjs
out/Release/node /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-http-imports.mjs
out/Release/node /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-chaining.mjs
out/Release/node /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-thenable.mjs
out/Release/node /home/mzasso/git/nodejs/node/test/es-module/test-esm-nowarn-exports.mjs
make[1]: *** [Makefile:308: jstest] Error 1
make: *** [Makefile:334: test] Error 2

After (18 failures)

=== release test-esm-loader ===
Path: es-module/test-esm-loader
(node:2968400) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
node:internal/process/esm_loader:19
    internalBinding('errors').triggerUncaughtException(
                              ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
+ actual - expected

+ Comparison {}
- Comparison {
-   code: 'ERR_MODULE_NOT_FOUND'
- }
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async file:///home/mzasso/git/nodejs/node/test/es-module/test-esm-loader.mjs:6:1 {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'nonexistent' imported from /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader.mjs
      at new NodeError (node:internal/errors:399:5)
      at packageResolve (node:internal/modules/esm/resolve:795:9)
      at moduleResolve (node:internal/modules/esm/resolve:844:20)
      at defaultResolve (node:internal/modules/esm/resolve:1069:11)
      at nextResolve (node:internal/modules/esm/hooks:638:28)
      at resolve (file:///home/mzasso/git/nodejs/node/test/fixtures/es-module-loaders/hooks-custom.mjs:76:10)
      at nextResolve (node:internal/modules/esm/hooks:638:28)
      at Hooks.resolve (node:internal/modules/esm/hooks:244:26)
      at MessagePort.handleSyncMessage (node:internal/modules/esm/worker:56:26)
      at [nodejs.internal.kHybridDispatch] (node:internal/event_target:738:20),
  expected: { code: 'ERR_MODULE_NOT_FOUND' },
  operator: 'rejects'
}

Node.js v20.0.0-pre
Command: out/Release/node --experimental-loader ./test/fixtures/es-module-loaders/hooks-custom.mjs /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader.mjs


=== release test-esm-loader-stringify-text ===
Path: es-module/test-esm-loader-stringify-text
(node:2968958) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
node:internal/process/promises:289
            triggerUncaughtException(err, true /* fromPromise */);
            ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
+ actual - expected

+ undefined
- 'ERR_INVALID_RETURN_PROPERTY_VALUE'
    at file:///home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-stringify-text.mjs:8:12
    at /home/mzasso/git/nodejs/node/test/common/index.js:448:15
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: undefined,
  expected: 'ERR_INVALID_RETURN_PROPERTY_VALUE',
  operator: 'strictEqual'
}

Node.js v20.0.0-pre
Command: out/Release/node --experimental-loader ./test/fixtures/es-module-loaders/string-sources.mjs /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-stringify-text.mjs


=== release test-esm-named-exports ===
Path: es-module/test-esm-named-exports
(node:2969120) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
custom-node:module:1
const $builtinInstance = $__get_builtin_hole_1672580803226("module");
                         ^

ReferenceError: $__get_builtin_hole_1672580803226 is not defined
    at custom-node:module:1:26
    at ModuleJob.run (node:internal/modules/esm/module_job:193:25)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

Node.js v20.0.0-pre
Command: out/Release/node --experimental-loader ./test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs /home/mzasso/git/nodejs/node/test/es-module/test-esm-named-exports.mjs


=== release test-esm-loader-obsolete-hooks ===
Path: es-module/test-esm-loader-obsolete-hooks
TAP version 13
Command: out/Release/node /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-obsolete-hooks.mjs
--- TIMEOUT ---


=== release test-esm-loader-chaining ===
Path: es-module/test-esm-loader-chaining
TAP version 13
# Subtest: ESM: loader chaining
    # Subtest: should load unadulterated source when there are no loaders
    ok 1 - should load unadulterated source when there are no loaders
      ---
      duration_ms: 225.710985
      ...
    # Subtest: should load properly different source when only load changes something
    not ok 2 - should load properly different source when only load changes something
      ---
      duration_ms: 222.6427
      failureType: 'testCodeFailure'
      error: |-
        The input did not match the regular expression /load passthru/. Input:

        'foo\nresolve passthru\n'

      code: 'ERR_ASSERTION'
      actual: |-
        foo
        resolve passthru

      operator: 'match'
      stack: |-
        Object.<anonymous> (file:///home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-chaining.mjs:50:12)
        async ItTest.run (node:internal/test_runner/test:548:9)
        async Promise.all (index 1)
        async Suite.run (node:internal/test_runner/test:798:7)
      ...
    # Subtest: should result in proper output from multiple changes in resolve hooks
    ok 3 - should result in proper output from multiple changes in resolve hooks
      ---
      duration_ms: 230.775636
      ...
    # Subtest: should respect modified context within resolve chain
    not ok 4 - should respect modified context within resolve chain
      ---
      duration_ms: 227.725052
      failureType: 'testCodeFailure'
      error: |-
        The input did not match the regular expression /bar/. Input:

        '42\nresolve 42\n'

      code: 'ERR_ASSERTION'
      actual: |-
        42
        resolve 42

      operator: 'match'
      stack: |-
        Object.<anonymous> (file:///home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-chaining.mjs:99:12)
        async ItTest.run (node:internal/test_runner/test:548:9)
        async Promise.all (index 3)
        async Suite.run (node:internal/test_runner/test:798:7)
      ...
    # Subtest: should accept only the correct arguments
    not ok 5 - should accept only the correct arguments
      ---
      duration_ms: 213.329172
      failureType: 'testCodeFailure'
      error: |-
        The input did not match the regular expression /^resolve arg count: 3$/m. Input:

        ''

      code: 'ERR_ASSERTION'
      actual: ''
      operator: 'match'
      stack: |-
        Object.<anonymous> (file:///home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-chaining.mjs:116:12)
        async ItTest.run (node:internal/test_runner/test:548:9)
        async Promise.all (index 4)
        async Suite.run (node:internal/test_runner/test:798:7)
      ...
    # Subtest: should result in proper output from multiple changes in resolve hooks
    ok 6 - should result in proper output from multiple changes in resolve hooks
      ---
      duration_ms: 214.385092
      ...
    # Subtest: should provide the correct "next" fn when multiple calls to next within same loader
    not ok 7 - should provide the correct "next" fn when multiple calls to next within same loader
      ---
      duration_ms: 239.430812
      failureType: 'testCodeFailure'
      error: |-
        Expected values to be strictly equal:

        1 !== 2

      code: 'ERR_ASSERTION'
      expected: 2
      actual: 1
      operator: 'strictEqual'
      stack: |-
        Object.<anonymous> (file:///home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-chaining.mjs:168:12)
        async ItTest.run (node:internal/test_runner/test:548:9)
        async Promise.all (index 6)
        async Suite.run (node:internal/test_runner/test:798:7)
      ...
    # Subtest: should use the correct `name` for next<HookName>'s function
    not ok 8 - should use the correct `name` for next<HookName>'s function
      ---
      duration_ms: 248.006465
      failureType: 'testCodeFailure'
      error: |-
        The input did not match the regular expression /next<HookName>: nextResolve/. Input:

        '42\nresolve 42\n'

      code: 'ERR_ASSERTION'
      actual: |-
        42
        resolve 42

      operator: 'match'
      stack: |-
        Object.<anonymous> (file:///home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-chaining.mjs:188:12)
        process.processTicksAndRejections (node:internal/process/task_queues:95:5)
        async ItTest.run (node:internal/test_runner/test:548:9)
        async Promise.all (index 7)
        async Suite.run (node:internal/test_runner/test:798:7)
      ...
    # Subtest: should throw for incomplete resolve chain, citing errant loader & hook
    not ok 9 - should throw for incomplete resolve chain, citing errant loader & hook
      ---
      duration_ms: 321.711678
      failureType: 'testCodeFailure'
      error: |-
        The input did not match the regular expression /resolve passthru/. Input:

        ''

      code: 'ERR_ASSERTION'
      actual: ''
      operator: 'match'
      stack: |-
        Object.<anonymous> (file:///home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-chaining.mjs:206:12)
        async ItTest.run (node:internal/test_runner/test:548:9)
        async Promise.all (index 8)
        async Suite.run (node:internal/test_runner/test:798:7)
      ...
    # Subtest: should NOT throw when nested resolve hook signaled a short circuit
    ok 10 - should NOT throw when nested resolve hook signaled a short circuit
      ---
      duration_ms: 333.012634
      ...
    # Subtest: should NOT throw when nested load hook signaled a short circuit
    ok 11 - should NOT throw when nested load hook signaled a short circuit
      ---
      duration_ms: 308.210943
      ...
    # Subtest: should allow loaders to influence subsequent loader resolutions
    not ok 12 - should allow loaders to influence subsequent loader resolutions
      ---
      duration_ms: 327.884519
      failureType: 'testCodeFailure'
      error: |-
        Expected values to be strictly equal:
        + actual - expected

        + 'node:internal/process/esm_loader:19\n' +
        +   "    internalBinding('errors').triggerUncaughtException(\n" +
        +   '                              ^\n' +
        +   '\n' +
        +   "Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'xxx' imported from /home/mzasso/git/nodejs/node/test/fixtures/es-module-loaders/\n" +
        +   '    at new NodeError (node:internal/errors:399:5)\n' +
        +   '    at packageResolve (node:internal/modules/esm/resolve:795:9)\n' +
        +   '    at moduleResolve (node:internal/modules/esm/resolve:844:20)\n' +
        +   '    at DefaultModuleLoader.defaultResolve (node:internal/modules/esm/resolve:1069:11)\n' +
        +   '    at DefaultModuleLoader.resolve (node:internal/modules/esm/loader:298:32)\n' +
        +   '    at DefaultModuleLoader.getModuleJob (node:internal/modules/esm/loader:150:34)\n' +
        +   '    at DefaultModuleLoader.import (node:internal/modules/esm/loader:252:10)\n' +
        +   '    at initializeHooks (node:internal/modules/esm/utils:122:65)\n' +
        +   '    at async setupESMWorker (node:internal/modules/esm/worker:37:13)\n' +
        +   '\n' +
        +   'Node.js v20.0.0-pre\n'
        - ''
      code: 'ERR_ASSERTION'
      expected: ''
      actual: |-
        node:internal/process/esm_loader:19
            internalBinding('errors').triggerUncaughtException(
                                      ^

        Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'xxx' imported from /home/mzasso/git/nodejs/node/test/fixtures/es-module-loaders/
            at new NodeError (node:internal/errors:399:5)
            at packageResolve (node:internal/modules/esm/resolve:795:9)
            at moduleResolve (node:internal/modules/esm/resolve:844:20)
            at DefaultModuleLoader.defaultResolve (node:internal/modules/esm/resolve:1069:11)
            at DefaultModuleLoader.resolve (node:internal/modules/esm/loader:298:32)
            at DefaultModuleLoader.getModuleJob (node:internal/modules/esm/loader:150:34)
            at DefaultModuleLoader.import (node:internal/modules/esm/loader:252:10)
            at initializeHooks (node:internal/modules/esm/utils:122:65)
            at async setupESMWorker (node:internal/modules/esm/worker:37:13)

        Node.js v20.0.0-pre

      operator: 'strictEqual'
      stack: |-
        Object.<anonymous> (file:///home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-chaining.mjs:268:12)
        process.processTicksAndRejections (node:internal/process/task_queues:95:5)
        async ItTest.run (node:internal/test_runner/test:548:9)
        async Promise.all (index 11)
        async Suite.run (node:internal/test_runner/test:798:7)
      ...
    # Subtest: should throw when the resolve chain is broken
    ok 13 - should throw when the resolve chain is broken
      ---
      duration_ms: 338.744039
      ...
    # Subtest: should throw for incomplete load chain, citing errant loader & hook
    not ok 14 - should throw for incomplete load chain, citing errant loader & hook
      ---
      duration_ms: 341.489149
      failureType: 'testCodeFailure'
      error: |-
        The input did not match the regular expression /load passthru/. Input:

        ''

      code: 'ERR_ASSERTION'
      actual: ''
      operator: 'match'
      stack: |-
        Object.<anonymous> (file:///home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-chaining.mjs:309:12)
        process.processTicksAndRejections (node:internal/process/task_queues:95:5)
        async ItTest.run (node:internal/test_runner/test:548:9)
        async Promise.all (index 13)
        async Suite.run (node:internal/test_runner/test:798:7)
      ...
    # Subtest: should throw when the load chain is broken
    ok 15 - should throw when the load chain is broken
      ---
      duration_ms: 256.978548
      ...
    # Subtest: should throw when invalid `specifier` argument passed to `nextResolve`
    ok 16 - should throw when invalid `specifier` argument passed to `nextResolve`
      ---
      duration_ms: 198.392358
      ...
    # Subtest: should throw when resolve hook is invalid
    ok 17 - should throw when resolve hook is invalid
      ---
      duration_ms: 316.932137
      ...
    # Subtest: should throw when invalid `context` argument passed to `nextResolve`
    ok 18 - should throw when invalid `context` argument passed to `nextResolve`
      ---
      duration_ms: 247.714191
      ...
    # Subtest: should throw when load hook is invalid
    ok 19 - should throw when load hook is invalid
      ---
      duration_ms: 268.620979
      ...
    # Subtest: should throw when invalid `url` argument passed to `nextLoad`
    ok 20 - should throw when invalid `url` argument passed to `nextLoad`
      ---
      duration_ms: 229.108731
      ...
    # Subtest: should throw when invalid `url` argument passed to `nextLoad`
    ok 21 - should throw when invalid `url` argument passed to `nextLoad`
      ---
      duration_ms: 206.813966
      ...
    # Subtest: should throw when invalid `context` argument passed to `nextLoad`
    ok 22 - should throw when invalid `context` argument passed to `nextLoad`
      ---
      duration_ms: 276.37821
      ...
    1..22
not ok 1 - ESM: loader chaining
  ---
  duration_ms: 491.227554
  failureType: 'subtestsFailed'
  error: '8 subtests failed'
  code: 'ERR_TEST_FAILURE'
  ...
1..1
# tests 1
# pass 0
# fail 1
# cancelled 0
# skipped 0
# todo 0
# duration_ms 497.194025
Command: out/Release/node /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-chaining.mjs


=== release test-esm-initialization ===
Path: es-module/test-esm-initialization
TAP version 13
# Subtest: ESM: ensure initialization happens only once
    # Subtest: <anonymous>
    not ok 1 - <anonymous>
      ---
      duration_ms: 113.990245
      failureType: 'testCodeFailure'
      error: |-
        Expected values to be strictly equal:

        1 !== 2

      code: 'ERR_ASSERTION'
      expected: 2
      actual: 1
      operator: 'strictEqual'
      stack: |-
        Object.<anonymous> (file:///home/mzasso/git/nodejs/node/test/es-module/test-esm-initialization.mjs:26:12)
        async ItTest.run (node:internal/test_runner/test:548:9)
        async Promise.all (index 0)
        async Suite.run (node:internal/test_runner/test:798:7)
      ...
    1..1
not ok 1 - ESM: ensure initialization happens only once
  ---
  duration_ms: 116.770862
  failureType: 'subtestsFailed'
  error: '1 subtest failed'
  code: 'ERR_TEST_FAILURE'
  ...
1..1
# tests 1
# pass 0
# fail 1
# cancelled 0
# skipped 0
# todo 0
# duration_ms 121.720359
Command: out/Release/node /home/mzasso/git/nodejs/node/test/es-module/test-esm-initialization.mjs


=== release test-code-cache ===
Path: parallel/test-code-cache
node:internal/modules/esm/worker:19
  workerData: { lock, syncCommPort },
                ^

TypeError: Cannot read properties of null (reading 'lock')
    at node:internal/modules/esm/worker:19:17
    at BuiltinModule.compileForInternalLoader (node:internal/bootstrap/loaders:334:7)
    at BuiltinModule.compileForPublicLoader (node:internal/bootstrap/loaders:270:10)
    at loadBuiltinModule (node:internal/modules/helpers:59:9)
    at Module._load (node:internal/modules/cjs/loader:898:20)
    at Module.require (node:internal/modules/cjs/loader:1128:19)
    at require (node:internal/modules/helpers:112:18)
    at Object.<anonymous> (/home/mzasso/git/nodejs/node/test/parallel/test-code-cache.js:19:3)
    at Module._compile (node:internal/modules/cjs/loader:1247:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1301:10)

Node.js v20.0.0-pre
Command: out/Release/node --expose-internals /home/mzasso/git/nodejs/node/test/parallel/test-code-cache.js


[06:27|% 100|+ 3793|-  18]: Done

Failed tests:
out/Release/node --experimental-loader ./test/fixtures/es-module-loaders/assertionless-json-import.mjs /home/mzasso/git/nodejs/node/test/es-module/test-esm-assertionless-json-import.js
out/Release/node --experimental-loader ./test/fixtures/es-module-loaders/example-loader.mjs /home/mzasso/git/nodejs/node/test/es-module/test-esm-example-loader.mjs
out/Release/node --experimental-import-meta-resolve /home/mzasso/git/nodejs/node/test/es-module/test-esm-import-meta-resolve.mjs
out/Release/node /home/mzasso/git/nodejs/node/test/es-module/test-esm-import-flag.mjs
out/Release/node --experimental-loader ./test/fixtures/es-module-loaders/hooks-custom.mjs /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader.mjs
out/Release/node /home/mzasso/git/nodejs/node/test/es-module/test-esm-initialization.mjs
out/Release/node --experimental-loader ./test/fixtures/es-module-loaders/loader-invalid-url.mjs /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-invalid-url.mjs
out/Release/node --loader ./test/fixtures/es-module-loaders/mock-loader.mjs /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-mock.mjs
out/Release/node /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-http-imports.mjs
out/Release/node --experimental-loader ./test/fixtures/es-module-loaders/string-sources.mjs /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-stringify-text.mjs
out/Release/node --experimental-loader ./test/fixtures/es-module-loaders/loader-side-effect.mjs --require ./test/fixtures/es-module-loaders/loader-side-effect-require-preload.js /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-side-effect.mjs
out/Release/node /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-chaining.mjs
out/Release/node --loader ./test/fixtures/es-module-loaders/hook-resolve-type.mjs /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-resolve-type.mjs
out/Release/node /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-thenable.mjs
out/Release/node --experimental-loader ./test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs /home/mzasso/git/nodejs/node/test/es-module/test-esm-named-exports.mjs
out/Release/node /home/mzasso/git/nodejs/node/test/es-module/test-esm-nowarn-exports.mjs
out/Release/node --expose-internals /home/mzasso/git/nodejs/node/test/parallel/test-code-cache.js
out/Release/node /home/mzasso/git/nodejs/node/test/es-module/test-esm-loader-obsolete-hooks.mjs

targos · 2023-01-01T14:25:42Z

There are still failures related to the console.log calls in test loaders. That's because of the worker.unref(), which prevents some messages from arriving before the process shuts down. We could add an artificial timeout in the test like I did in JakobJingleheimer/worker-atomics#4, or go back to fs.writeSync for this use case.
console.log is not broken in general with this approach.

GeoffreyBooth · 2023-01-02T00:19:59Z

@targos I rebased JakobJingleheimer/feat/esm_off-thread-loaders off of main now that nodejs#46016 has landed, and then I rebased this accordingly so that you don’t have conflicts. I now see only 17 failures on this branch, compared with 16 on the base. The only new failure is test/es-module/test-esm-initialization.mjs.

However one of the failures on this branch is a timeout, which I’ve grown to dread from the other branch; on the other branch at least, those have always meant that an exception was being thrown inside the worker that never got transferred up to the main thread to be re-thrown there to crash the process. I don’t know if maybe there could be other reasons for a timeout now that the main thread isn’t locked, but ideally we’d fix the timeout (even if the test in question still fails) before merging this in. The timing out test is test/es-module/test-esm-loader-obsolete-hooks.mjs; to cause the timeout directly, rather than inside a spawned child process, run ./node --no-warnings --throw-deprecation --loader ./test/fixtures/es-module-loaders/hooks-obsolete.mjs ./test/fixtures/print-error-message.js.

Also I’m wondering if we can separate the refactor to use postMessage from the unlocking of the main thread. If those can be separate commits or PRs, then I’d think we could benchmark sharedArrayBuffer vs postMessage in isolation. I think postMessage is the more understandable API, but sharedArrayBuffer was originally chosen because the expectation was that it was significantly faster.

targos · 2023-01-02T10:28:57Z

I think there are the same test failures with e46ff62 alone and with b97e08e (including the timeout you told me about). I'm stopping for now. Maybe I'll investigate the timeout later.

JakobJingleheimer

Thanks! This looks very promising and far less arcane than Atomics

JakobJingleheimer · 2023-01-03T11:21:25Z

lib/internal/modules/esm/worker.js

    // Send the method response (or exception) to the main thread
    try {
-      TypedArrayPrototypeSet(data, serializedResponse);
+      syncCommPort.postMessage(response);


I'm pretty sure the cause of the huge performance difference between SharedArrayBuffer and postMessage() is because with the SAB, it's a single shared memory space (so the transfer across threads is merely a context switch and nothing is moved/copied); but with postMessage(), when only the first arg is used, the response is copied (which is relatively expensive, on top of consuming double the space).

I think if you instead transfer the response via the 2nd param, we'll see more similar (better) performance with postMessage()

The 2nd param of postMessage can only be used with transferable objects. response is not transferable. I still haven't seen any benchmark or perf numbers so I'm not really inclined to talk about it just based on assumptions.

lib/internal/modules/esm/hooks.js

use postMessage from the main thread instead of atomics to signal the worker

GeoffreyBooth force-pushed the feat/esm_off-thread-loaders branch from c8ea77a to 2e60363 Compare January 1, 2023 23:24

GeoffreyBooth force-pushed the esm-off-thread-unlock-worker branch 2 times, most recently from b07e12a to a04dcae Compare January 2, 2023 00:08

targos force-pushed the esm-off-thread-unlock-worker branch from a04dcae to b97e08e Compare January 2, 2023 10:24

targos force-pushed the esm-off-thread-unlock-worker branch from b97e08e to 1a7cbd4 Compare January 2, 2023 10:29

JakobJingleheimer reviewed Jan 3, 2023

View reviewed changes

GeoffreyBooth force-pushed the feat/esm_off-thread-loaders branch 2 times, most recently from dbd039f to 1e6c05e Compare January 7, 2023 21:54

targos force-pushed the esm-off-thread-unlock-worker branch 2 times, most recently from 3f10022 to f224714 Compare January 8, 2023 16:05

targos added 3 commits January 8, 2023 20:31

fix doc lint

33b02cb

unlock worker thread

fe9111e

use postMessage from the main thread instead of atomics to signal the worker

use syncCommPort to exchange data too

88eac69

GeoffreyBooth force-pushed the esm-off-thread-unlock-worker branch from f224714 to 88eac69 Compare January 9, 2023 05:02

GeoffreyBooth merged commit 88eac69 into JakobJingleheimer:feat/esm_off-thread-loaders Jan 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Esm off thread unlock worker #2

Esm off thread unlock worker #2

targos commented Jan 1, 2023

targos commented Jan 1, 2023

GeoffreyBooth commented Jan 2, 2023 •

edited

targos commented Jan 2, 2023

JakobJingleheimer left a comment

JakobJingleheimer Jan 3, 2023

targos Jan 3, 2023

Esm off thread unlock worker #2

Esm off thread unlock worker #2

Conversation

targos commented Jan 1, 2023

targos commented Jan 1, 2023

GeoffreyBooth commented Jan 2, 2023 • edited

targos commented Jan 2, 2023

JakobJingleheimer left a comment

Choose a reason for hiding this comment

JakobJingleheimer Jan 3, 2023

Choose a reason for hiding this comment

targos Jan 3, 2023

Choose a reason for hiding this comment

GeoffreyBooth commented Jan 2, 2023 •

edited