Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support Bun #70

Closed
wants to merge 1 commit into from
Closed

Conversation

AriPerkkio
Copy link
Member

@AriPerkkio AriPerkkio commented Sep 13, 2023

Related to vitest-dev/vitest#4111

  • node:worker_threads are unstable on Bun and crash under heavy load
  • node:worker_threads get stuck on Bun if there are while(true) {} loops inside the worker
  • Bun's Workers are unstable, leak memory and crash under heavy load
  • Bun does not open MessagePorts automatically, port.start() must be called manually
  • Bun does not start node:child_process immediately like NodeJS does. It also does not emit spawn event. Manual work-arounds are required.
# Test how fast NodeJS can spin up and tear down 5000 worker_threads, 8 at a time on 8-CPU machine
$ node isolate-benchmark.mjs --threads 7 --rounds 5000
Options: { THREADS: 7, ROUNDS: 5000, IS_BUN: false } 

Native node:worker_threads | START
Native node:worker_threads | END   24622 ms

# Test how fast Bun can spin up and tear down 5000 Workers, 8 at a time on 8-CPU machine
$ bun isolate-benchmark.mjs --threads 7 --rounds 5000
Options: {
  THREADS: 7,
  ROUNDS: 5000,
  IS_BUN: true
} 

Native Bun workers | START
FATAL ERROR: JavaScript garbage collection failed because thread_get_state returned an error (268435459). This is probably the result of running inside Rosetta, which is not supported.
/Users/jarred/actions-runner/_work/WebKit/WebKit/Source/WTF/wtf/posix/ThreadingPOSIX.cpp(497) : size_t WTF::Thread::getRegisters(const WTF::ThreadSuspendLocker &, WTF::PlatformRegisters &)

Sometimes Bun crashes with libc++abi: Pure virtual function called! Abort trap: 6 error.

@@ -38,6 +38,12 @@ parentPort!.on('message', (message: StartupMessage) => {
useAtomics =
process.env.PISCINA_DISABLE_ATOMICS === '1' ? false : message.useAtomics

if (useAtomics && process.versions.bun) {
const error = 'useAtomics cannot be used with Bun at the moment.'
Copy link
Member Author

@AriPerkkio AriPerkkio Sep 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workers are unable to terminate when using bun. I think the while loops below are causing it. oven-sh/bun#4134

import { isMainThread, Worker } from "node:worker_threads";
import { fileURLToPath } from "node:url";

if (isMainThread) {
  console.log("[main] running");
  const worker = new Worker(fileURLToPath(import.meta.url));

  await new Promise((r) => setTimeout(r, 1_000));
  const timeout = setTimeout(() => console.error("Timeout"), 2_000);

  console.log("[main] terminating");
  await worker.terminate();
  clearTimeout(timeout);
  console.log("[main] terminated");
} else {
  console.log("[worker] running");

  while (true) {}
}
ari ~/repro  $ node worker-threads.mjs 
[main] running
[worker] running
[main] terminating
[main] terminated

ari ~/repro  $ bun worker-threads.mjs 
[main] running
[worker] running
[main] terminating
Timeout
<stuck here>

@AriPerkkio AriPerkkio changed the title feat: support bun feat: support Bun Sep 13, 2023
@AriPerkkio AriPerkkio force-pushed the feat/support-bun branch 2 times, most recently from 529b5ff to 69d2828 Compare September 16, 2023 13:40
@AriPerkkio
Copy link
Member Author

Let's wait for fixes from Bun before moving on. Adding work-arounds just for Bun doesn't feel right.

oven-sh/bun#5709

AriPerkkio added a commit to AriPerkkio/tinypool that referenced this pull request Sep 26, 2023
- Instead of using Node APIs like in tinylibs#70, use Bun's native Workers API
AriPerkkio added a commit to AriPerkkio/tinypool that referenced this pull request Sep 27, 2023
- Instead of using Node APIs like in tinylibs#70, use Bun's native Workers API
AriPerkkio added a commit to AriPerkkio/tinypool that referenced this pull request Sep 27, 2023
- Instead of using Node APIs like in tinylibs#70, use Bun's native Workers API
@AriPerkkio
Copy link
Member Author

Adding Bun support for Node APIs like node:worker_threads and node:child_process is not fun. I would not want to add Bun related work-arounds that affect Node users.

Let's instead add support for Bun's native Worker: #73

@AriPerkkio AriPerkkio closed this Sep 27, 2023
@AriPerkkio AriPerkkio deleted the feat/support-bun branch September 27, 2023 18:00
AriPerkkio added a commit to AriPerkkio/tinypool that referenced this pull request Sep 28, 2023
- Instead of using Node APIs like in tinylibs#70, use Bun's native Workers API
AriPerkkio added a commit to AriPerkkio/tinypool that referenced this pull request Oct 1, 2023
- Instead of using Node APIs like in tinylibs#70, use Bun's native Workers API
AriPerkkio added a commit to AriPerkkio/tinypool that referenced this pull request Oct 19, 2023
- Instead of using Node APIs like in tinylibs#70, use Bun's native Workers API
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant