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

[Discussion] tinylibs/tinylet or jimmywarting/await-sync? #4

Open
jcbhmr opened this issue Jul 6, 2023 · 8 comments
Open

[Discussion] tinylibs/tinylet or jimmywarting/await-sync? #4

jcbhmr opened this issue Jul 6, 2023 · 8 comments

Comments

@jcbhmr
Copy link
Member

jcbhmr commented Jul 6, 2023

@Aslemammad @jimmywarting

I just found out that there's https://github.com/jimmywarting/await-sync and that got me thinking... It'd be cool if there was one way of doing things with regard to synchronous worker magic. Right now there is:

I can acknowledge that some of the more low-level Node.js-specific ones are in a "different class" but there's https://github.com/jimmywarting/await-sync and https://github.com/tinylibs/tinylet which seem to do the same thing(ish)...

If possible, I think it'd be great for either this package or https://github.com/jimmywarting/await-sync to be "the one". I'm perfectly OK with deprecating (or yanking since this package has <10 weekly DLs and no dependants) in favor of https://github.com/jimmywarting/await-sync and contributing my changes there, or it could go the other way around. 🤷‍♂️

TLDR: I think that these two packages overlap enough that I'd like to combine them. Is this a good idea? 🤔 @jimmywarting thoughts? I don't know if I'm overstepping etiquette or anything here.

@jcbhmr

This comment was marked as off-topic.

@Aslemammad
Copy link
Member

Competition and Collaboration would always be the case. I'd like to have you and your package in tinylibs and at the same time compete and collaborate with other similar packages! So please continue you amazing work since I believe this is a neat idea and a neat API you're developing for tinylibs.

@jimmywarting
Copy link

jimmywarting commented Jul 7, 2023

If you want to contribute and ship the missing equivalent greenlet function into my pkg then i'm willing to accept some help. my pkg dose not use any NodeJS specific stuff like node:worker_threads or node:buffer (to be more cross env friendly) and not having to code platform specific things using node:worker_thread instead of using web worker. instead mine loads another whatwg worker spec compatible dependency (only in nodejs) and uses the same underlying api. using https://github.com/jimmywarting/whatwg-worker instead. maybe you want to use this instead, and remove redlet-node.js & greenlet-node.js?

whatever you decide to do is up to you...
before you making any decision i would suggest that you try and measure the performance of both... if it turns out one is a lot faster than something else.
don't test it with remote http calls like fetch, instead use stuff like fs or something else.
also test doing repeated calls. like doing a warmup and calling the same function over and over again. my solution is persistent. see: https://github.com/jimmywarting/await-sync#misc

@jcbhmr
Copy link
Member Author

jcbhmr commented Jul 7, 2023

my pkg dose not use any NodeJS specific stuff like node:worker_threads or node:buffer (to be more cross env friendly) and not having to code platform specific things using node:worker_thread instead of using web worker. instead mine loads another whatwg worker spec compatible dependency (only in nodejs) and uses the same underlying api. using https://github.com/jimmywarting/whatwg-worker instead. maybe you want to use this instead, and remove redlet-node.js & greenlet-node.js?

Yeah, I originally wanted to do that, but I realized it could "specialize" the function for Node.js to hopefully avoid paying shim perf price. I figured the specialized implementation was worth it in this case, as long as it achieves rough feature-parity with the non-Node.js impl that needs JSON-like serialization (it doesn't YET; I still need to buff up that JSON-like serialization)

@jcbhmr
Copy link
Member Author

jcbhmr commented Jul 7, 2023

As for benchmarks, here's what I got:

image

From this code: https://github.com/tinylibs/tinylet/blob/83eee3371feef310a1ec13b20bb0ed299d78c063/test/redlet-node.bench.js

I note that there might be something I'm missing with the "await-sync: create + invoke worker" one. I don't know if I'm using the library wrong or what 😵 2 ops/sec doesn't sound right

image
image
vs "redlet: create + invoke worker" which is somehow wayyyy far and away better 🤣 idk what's happening 4000 vs 2 ops/sec is massive.
image

@jcbhmr
Copy link
Member Author

jcbhmr commented Jul 7, 2023

Here's another run with the same lopsided benchmark result... 🆘😱😩 what is happening
image

@jcbhmr
Copy link
Member Author

jcbhmr commented Jul 7, 2023

General comments on the benchmarking stuff though is that redlet() seems WAYYYYY faster. Here's a comparison chart that I collected from ☝☝ bench results:

code excerpt for your reading pleasure
bench.add("redlet: create only", () => {
  redlet(() => {});
});

bench.add("await-sync: create only", () => {
  awaitSync(() => new Uint8Array(1));
});

bench.add("redlet: create + invoke worker", () => {
  const f = redlet(() => {});
  f();
});

bench.add("await-sync: create + invoke worker", () => {
  const f = awaitSync(() => new Uint8Array(1));
  f();
});

{
  const f = redlet(() => {});
  bench.add("redlet: invoke worker only", () => {
    f();
  });
}

{
  const f = awaitSync(() => new Uint8Array(1));
  bench.add("await-sync: invoke worker only", () => {
    f();
  });
}
redlet await-sync
create only 23x 1x
create + invoke worker 1611x 1x
invoke worker only 22x 1x

REMEMBER THAT THIS IS JUST RAW f = ()=>{}; f() function calls though! This isn't real world serialization or anything. I need to do those benchmarks yet 😊

@jimmywarting
Copy link

jimmywarting commented Jul 7, 2023

I included the tinylet into my own benchmark code...

import { readFileSync } from 'node:fs'
import { Buffer } from 'node:buffer'
import { createWorker } from '../mod.js'
import makeSynchronous from 'make-synchronous'
import { createSyncFn } from 'synckit'
import { redlet } from 'tinylet'

// the worker path must be absolute
const workerPath = new URL('./synkit-worker.js', import.meta.url).toString().slice(7)
const awaitSync = createSyncFn(workerPath, {})

const sin = makeSynchronous(async path => {
  const fs = await import('fs/promises')
  return fs.readFile(new URL(path))
})

const path = new URL('./bench.js', import.meta.url) + ''

const jim = createWorker()(async path => {
  const fs = await import('fs/promises')
  return fs.readFile(new URL(path))
})

let i

// Runs in a worker thread and uses Atomics.wait() to block the current thread.
const redletReader = redlet(async (path) => {
  const fs = await import('fs/promises')
  return fs.readFile(new URL(path))
})

i = 100
console.time('fs.readFileSync')
while (i--) readFileSync(new URL(path))
console.timeEnd('fs.readFileSync')

globalThis?.gc()

i = 100
console.time('redletReader')
while (i--) redletReader(path)
console.timeEnd('redletReader')

globalThis?.gc()

i = 100
console.time('synkit')
while (i--) awaitSync(path)
console.timeEnd('synkit')

globalThis?.gc()

i = 100
console.time('await-sync')
while (i--) jim(path)
console.timeEnd('await-sync')

globalThis?.gc()

i = 100
console.time('make-syncronous')
while (i--) sin(path)
console.timeEnd('make-syncronous')

tried reading the same file 100 times:

fs.readFileSync: 1.285ms
redletReader: 41.637ms
synkit: 11.118ms
await-sync: 24.936ms
make-syncronous: 3.569s

same thing for reading it only 10 times:

fs.readFileSync: 0.217ms
redletReader: 29.833ms
synkit: 14.302ms
await-sync: 5.795ms
make-syncronous: 377.195ms

and 400 times:

fs.readFileSync: 4.994ms
redletReader: 78.388ms
synkit: 38.584ms
await-sync: 92.132ms
make-syncronous: 14.265s

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

No branches or pull requests

3 participants