Open jcbhmr opened 1 year ago
IMO (this is biased) I like the idea of this tinylet package.
But I'd be happy to consolidate on the existing solution at https://github.com/jimmywarting/await-sync and make another greenlet2 or whatever package for the greenlet() stuff I want to do (worker pooling, caching, timeouts, etc)
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.
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
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)
As for benchmarks, here's what I got:
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
vs "redlet: create + invoke worker" which is somehow wayyyy far and away better 🤣 idk what's happening 4000 vs 2 ops/sec is massive.
Here's another run with the same lopsided benchmark result... 🆘😱😩 what is happening
General comments on the benchmarking stuff though is that redlet() seems WAYYYYY faster. Here's a comparison chart that I collected from ☝☝ bench results:
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 😊
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
@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.