Skip to content

Commit

Permalink
push out env var access
Browse files Browse the repository at this point in the history
  • Loading branch information
znewsham committed Sep 20, 2023
1 parent 1e00960 commit 2d44319
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
4 changes: 2 additions & 2 deletions fibers_sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,9 @@ function setupAsyncHacks(Fiber) {
}
}

const logUseFibersLevel = +(process.env.ENABLE_LOG_USE_FIBERS || 0);
const { LOG_USE_FIBERS_INCLUDE_IN_PATH } = process.env;
function logUsingFibers(fibersMethod) {
const logUseFibersLevel = +(process.env.ENABLE_LOG_USE_FIBERS || 0);

if (!logUseFibersLevel) return;

Expand All @@ -98,7 +99,6 @@ function setupAsyncHacks(Fiber) {
return;
}

const { LOG_USE_FIBERS_INCLUDE_IN_PATH } = process.env;
const stackFromError = new Error(`[FIBERS_LOG] Using ${fibersMethod}.`).stack;

if (
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "fibers",
"version": "5.0.2-6",
"version": "5.0.2-7",
"description": "Cooperative multi-tasking for Javascript",
"keywords": [
"fiber",
Expand Down

6 comments on commit 2d44319

@theosp
Copy link

@theosp theosp commented on 2d44319 Oct 22, 2023

Choose a reason for hiding this comment

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

Hey, can you help me make sense of your branch ? Did you manage to get Fibers work on node v16?

@znewsham
Copy link
Author

Choose a reason for hiding this comment

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

The short answer is yes, it works on node 16 and 18. But this fork is designed to also designed to play nice with asynclocalstorage, and meteors environment variables and fiber pool.

@theosp
Copy link

@theosp theosp commented on 2d44319 Oct 22, 2023

Choose a reason for hiding this comment

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

Thanks for the quick reply 🙏

18 EOL is Apr 2025 - it is a really good amount of time.

I am trying to construct an upgrade strategy for my own Meteor app. The last thing I want is to waste resources on endless redundant rewrites in a mature codebase. I am shocked that the community ended up going in the direction of rewrites.

With this your plan is to stay on Meteor's 2.x branch?
I've seen mentions around that the use of CORO_PTHREAD might have a negative impact on performance, did you experience anything like that?
Any resource on how to make use of your fork?

@znewsham
Copy link
Author

Choose a reason for hiding this comment

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

We're moving off of meteor for a variety of reasons, we built a converter we're calling meteor-lite which converts most meteor packages to clean npm packages then we just use esbuild. It uses about 30% less memory in production the prod build is 4x faster and dev rebuilds are faster too (though dev memory is still high). But best of all there is no magic - things work exactly as you'd expect in a node app.

In terms of performance, yeah there is for sure a big one. The combo of the threads and the cost of the async local storage is pretty bad, we knew this going in but what burned us is that the meteor 2.8 version of mongo uses code which means there are 3x as many fiber yields for db operations. Extra fun was we assumed the perf cost would scale linearly. In node 14 it seems to take about 2 seconds/1,400,000 yields in dev and various configurations of prod (e.g. different aws nodes). With this version of fibers that time went up to 45 seconds on 2ghz cpus and around 17seconds on zd instances (4ghz). We mitigated by reducing the number of fiber yields in db operations and as we move off fibers the cost will decrease. While this sounds really bad, we only observed it in a tiny % of our method calls. Generally where we need to iterate over thousands of documents and not do much to them (think server side accumulator or something)

I don't think there's a way to avoid the rewrites sadly, but we wanted something that will allow us to move to modern node builds while we migrate vs a hard swap. So our meteor apps are currently mostly fibers with some clean promise code. New code is written as clean promise code and where it needs to interact with fibers we use a helper to spawn a fiber.

@theosp
Copy link

@theosp theosp commented on 2d44319 Oct 22, 2023

Choose a reason for hiding this comment

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

Thanks for the detailed answer 🙏

Your Meteor-lite is closed source, right?

@znewsham
Copy link
Author

@znewsham znewsham commented on 2d44319 Oct 22, 2023

Choose a reason for hiding this comment

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

I worked on the initial version in my free time so it's open source and is available Here but when Qualia decided to move in that direction we close sourced it, for a number of reasons none that are necessary permanent, mostly to avoid accidents and allow us to move faster without caring about general use cases. In any case that repo is quite out of date now. I'll ask next week if we can open source it.

I will say, it's not for the faint of heart - we use around 400 packages and only needed to make manual modifications to about 30 of them, but in many cases those changes were a PITA. The changes to meteors core packages for example (minimongo in particular has what should be circular referenced all over the damn place) you also need somewhere to put all the assets when you're done, we self host a verdaccio instance.

Please sign in to comment.