-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
WIP: Fix #6736: Update bun to latest version in CI #6737
base: master
Are you sure you want to change the base?
Conversation
It seems tests were already failing in the same manner as described in #6736 under bun 1.1.18, for instance: https://github.com/pubkey/rxdb/actions/runs/12634621221/job/35202766323#step:10:186 Increasing the timeout globally, e.g running with
@pubkey, given the pre-existing timeouts in test runs under 1.1.18, I suspect the slowness and associated failures under this runtime have been here for quite some time. In fact, I cannot seem to find a github workflow where the test suite passed. It was already failing two months ago, like here: https://github.com/pubkey/rxdb/actions/runs/11912718263/job/33196921290#step:10:197 Even with a 50s timeout, I'm still finding tests that fail because they are too slow, thus I'm not sure what the best course of action is. Maybe set a 30s timeout and simply skip the tests that fail above that limit, to still have some coverage? What do you think? |
8a1f81c
to
347cc5f
Compare
Still times out. Please run it locally first and ensure it works on your device. |
With the last commits and a 60s timeout, it runs on my device:
I had to disable a dozen slow tests, fix an actual comparison issue, disable a test crashing for an unknown reason, and disable compressionstream related test (the API isn't implemented in bun). In comparison to the standard node runtime test suite, bun takes 3-4x more times, and misses ~140 tests:
Some steps/tests are an under of magnitude slower under bun, at least on my computer:
I'm waiting for a CI run to see if it passes (or at least runs more tests than last time before crashing). As far as I can tell, the slowness under bun is I/O bound, because the CPU were mostly idle during test runs. And by the way I'm not saying we should merge this in its current state, but a (mostly) working test suite is an improvement given the prior situation and if the CI confirms my local runs, maybe we can decide how to procede next? |
These tests run on the in-memory indexeddb. They should neither be slow nor commented out. Lets check the CI. |
So, the CI run took 10m, but all the 974 tests passed. How would you like to proceed? |
We need to fix these performance problems. The tests run on in-memory storage only. There is something going wrong. Also having a 10min run for this should not be the case. The in-memory tests in node.js in fast mode take about 20 seconds. |
Agreed. I've tried several profiling tools to try to pinpoint the culprit:
If you have any pointers to profile performance under bun/motcha, I can dedicate a few more hours to this. Otherwise, I think I'm going to stop the task there as it's way beyond my skillset and what I assumed to be the original task ("Update Bun.js to the latest version while ensuring the tests still work"). The bun test suite didn't work in the first place, even under bun 1.1.18. It's been failing for at least two months, and I suspect it was the case even before that. |
Maybe we should switch from using dexie+in-memory-indexeddb to just use the memory RxStorage for the bun tests. This would make it easier to find out why it is slow and where exactly it is slow. |
Maybe I did it wrong, but with the memory storage (see the last commit), tests run in 4 minutes on my machine (versus 7 before), and yields 5 new failure. It's not blazing fast, but it's still faster:
|
Hey, could you fix these issues? Are you still working on this? |
The test suite was passing under dexie+memory, switching to memory storage as you recommended didn't really speed up the CI, but did break new tests for unknown reasons. I don't think it makes sense to fix these until we understand why the performance is so bad under bun. You said that:
How does it make it easier to pinpoint the root cause? I am unable to debug the performance issues without additional pointers. |
Cf #6736, Opening this now to trigger a test run in CI and see how it goes, I expect the Dexie suite to fail under bun 1.1.42
Todos