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

Performance: Don't block the event loop #294

Closed
js-kyle opened this issue Nov 29, 2018 · 18 comments
Closed

Performance: Don't block the event loop #294

js-kyle opened this issue Nov 29, 2018 · 18 comments

Comments

@js-kyle
Copy link
Contributor

js-kyle commented Nov 29, 2018

TLDR: You should make sure you never block the Event Loop. Node is fast when the work associated with each client at any given time is "small". In other words, each of your JavaScript callbacks should complete quickly, to avoid performance impacts.

Otherwise: The throughput (requests/second) of your server will suffer.

Suggested resources:

@js-kyle js-kyle changed the title Performance: Don't block the event look Performance: Don't block the event loop Nov 29, 2018
@TheHollidayInn
Copy link
Contributor

Can I take this?

@goldbergyoni
Copy link
Owner

Ideas:

  • Monitor the event loop block and bake into your monitoring
  • Mitigation techniques when having CPU intensive tasks: run the heavy tasks offline, other 3-4 techniques here
  • Use setImmediate to break long CPU-bounded tasks into multiple smaller tasks that wait in the queue for execution
  • Code example for detection:
const blocked = require('blocked-at');

const greedyFunction = () =>{
    const end = Date.now() + 2000;
            while (Date.now() < end) {
            eval("1+2+3");
        } 
    console.log(`Ending a greedy function`);
}

const handleBlockedLoop = (time, stack) => {
    console.log(`Blocked at ${stack} for ${time} ms`);
    //TODO: export to Promotheus here
}

const {stop} = blocked(handleBlockedLoop, {});
setImmediate(greedyFunction);
console.log(`Program finished`);

Other ideas?

@goldbergyoni
Copy link
Owner

@TheHollidayInn

Sure, with pleasure. Let's just first exchange some ideas here?

@TheHollidayInn
Copy link
Contributor

Yeah, definitely!

Few questions:

  • Do you have a recommended way to monitor? I'm guessing monitoring CPU usage/cycles would be helpful in tracking these down
  • Should we recommend queue/worker system for offloading cpu tasks?
  • "..JavaScript callbacks should complete quickly, to avoid performance impacts." can we add a snippet including ensuring your promises and awaited tasks end quickly - this is discussed in the link above.
  • What do you think of including this express example: https://stackoverflow.com/questions/51495928/how-does-node-process-concurrent-requests. I think this is a common pattern where you may linger tasks after responding to the users requests

@goldbergyoni
Copy link
Owner

goldbergyoni commented Dec 4, 2018

@TheHollidayInn My thoughts:

  • How to monitor? there are many npm packages that do this, search "event loop block", my code example above contains one of them. You can't monitor the CPU to detect event loop blocking because other things might keep the CPU busy so you're monitoring system will shout a false-positive or worst the loop might get blocked but the CPU in overall won't be overloaded (e.g. only 1 core out of 4 will be busy). You have to check for this specifically. Also worth mentioning that all APM products provide this functionality, I believe that PM2 also

  • Recommend queue? we absolutely need to recommend alternatives. I would prioritize the potential remedy: (1) offline work - prepare it in advance like a night job (2) *if not executed in all requests, run it in a separate process (3) break it into multiple smaller tasks, queue might be the technique to achieve (4) use different programming languages (5) use threads (6) others?

  • Async code end quickly - could you please clarify, how is this related to event loop? for example, if my promise fetches rows from DB which respond slowly this will not block the loop

-Which express example? I saw many messages there

Remember that our main goal is to curate/summarize content, hence all the information we gathered here should be propritized and only the gist should be included in the TLDR, some of the others should be part of the "more info". What we lack here is good bibilopgraphy/posts on how the event loop works and what happen when it's blocked

@TheHollidayInn
Copy link
Contributor

@i0natan Thanks!

  • Ah, I see. I'll look more into your library. I see this is similar to finding memory leaks even when your memory is not maxed
  • This list looks good. Is (1) a Cron Job? I'll also investigate other options. It seems from the article above we want to delegate offline or to a different language as the article recommends staying away from Worker Pools if possible and keeping the client connection small.
  • Nvm about the async. I reread and looked elsewhere and I misunderstood the affect for async/await on the event loop.

As for the Express example, I see this a lot in code I work on:

    app.use('/index', function(req, res, next) {
        console.log("hello index routes was invoked");

        readImage("path", function(err, content) {
            status = "Success";
            if(err) {
                console.log("err :", err);
                status = "Error"
            }
            else {
                console.log("Image read");
            }
            return res.send({ status: status });
        });

        var a = 4, b = 5;
        console.log("sum =", a + b);
    });

Where users run code after the Express response. I'm guessing as long as they are not using synchronous actions then this is okay and doesn't block the event loop, but it seems in condtriction to Node is fast when the work associated with each client at any given time is "small". Although the above example is easy.

Anyway, I'm not sure if it matters, but wonted to bring it up here.

@TheHollidayInn
Copy link
Contributor

Following up. I found this paragraph from: https://nodejs.org/en/docs/guides/dont-block-the-event-loop/

You should make sure you never block the Event Loop. In other words, each of your JavaScript callbacks should complete quickly. This of course also applies to your await's, your Promise.then's, and so on.

So I think the example ...my promise fetches rows from DB which respond slowly this will not block the loop.. would be an issue for the Event Loop.

@goldbergyoni
Copy link
Owner

goldbergyoni commented Dec 13, 2018

@TheHollidayInn Hey, thanks for pushing. My thoughts below.

  • Callback - I wouldn't discuss or include callback in any example, their past 😄

  • I see this is similar to finding memory leaks even when your memory is not maxed

Yes, similar

  • Is (1) a Cron Job?

Yes, for example, there are other methods

  • Fetching rows from DB - absolutely won't affect the event loop, this operation is executed within the DB and the event loop in the interim is moving to process other requests. This is the essence of the event loop. See this video: https://www.youtube.com/watch?v=8aGhZQkoFbQ

  • Another interesting point which other rarely cover - how sensitive is the event loop: should I execute a 100 cycles loop with some few if/else statement, will this dramatically affect the users? what exactly is a CPU-intensive task? many developers struggle to realize when their code becomes a block

  • A recommendation for a great resource: https://jsblog.insiderattack.net/event-loop-best-practices-nodejs-event-loop-part-5-e29b2b50bfe2

Whenever you're ready, before PRing, I suggest sharing here a best practice brief:

  • Your 2-3 main messages which will be included in the TLDR (e.g. Blocking the event loop means affecting 2000 online users, avoid CPU intensive task like XYZ, constantly monitor the event loop using ABC)
  • Overall TOC - messages you want to include in the read-more section
  • Bibliography - 2-3 great articles from which you draw inspiration and include quotes

@TheHollidayInn
Copy link
Contributor

Alright, finished reading through that series - it was amazing. Here is my first attempt and a very rough draft:

TLDR; Avoid using syncronous and other Event Loop blocking functions.

Node handles the Event Loop on a single thread rotating through multiple queues. Syncronous functions (usually ending with Sync) like readFileSync will prevent Node from moving through its phases thus blocking the Event Loop. Using too many process.nextTick calls or creating an infinite callback loop is another way to block the event loop. Use process.nextTick sparingly and carefully. Other CPU intensive tasks, such as DNS Lookup or Crypto will be offload to lubuv, take up other threads and starve new CPU intensive tasks. Use libraries such as loopbench or blocked-at to monitor your Event Loop and diagnose areas where the loop is being blocked.

Image of the Event Loop

1*aU5dr98pxTsZ4AMfnA6lNA.png

Here's a good rule of thumb for keeping your Node server speedy: Node is fast when the work associated with each client at any given time is "small".
Don't Block the Event Loop (or the Worker Pool) | Node.js

Most people fail their first few NodeJS apps merely due to the lack of understanding of the concepts such as the Event Loop, Error handling and asynchrony
Event Loop Best Practices — NodeJS Event Loop Part 5

@VinayaSathyanarayana
Copy link
Contributor

Nice to see progress on this topic that I introduced in
(#256 (comment)).
Thanks @i0natan @TheHollidayInn

We should add notes on how to detect/monitor - Some ideas from npmjs
https://www.npmjs.com/package/blocked
https://www.npmjs.com/package/blocked-at
https://www.npmjs.com/package/event-loop-stats
https://www.npmjs.com/package/event-loop-lag

@goldbergyoni
Copy link
Owner

@TheHollidayInn Thanks for improving, "great books are not written they are re-written" :]

It will take me two days more to get to review this.

@js-kyle, @VinayaSathyanarayana brought a good point, we should credit the people who suggested the origin ideas, any idea how to manage this?

@VinayaSathyanarayana Absolutely need to provide example, though I would choose 2-3 (all of them are 10-30 LOC and pretty similar)

@goldbergyoni
Copy link
Owner

(Message to Bot below)

/remind me in 2 days

@reminders reminders bot added the reminder label Jan 14, 2019
@reminders
Copy link

reminders bot commented Jan 14, 2019

@i0natan set a reminder for Jan 16th 2019

@reminders reminders bot removed the reminder label Jan 16, 2019
@reminders
Copy link

reminders bot commented Jan 16, 2019

👋 @i0natan,

@goldbergyoni goldbergyoni self-assigned this Jan 19, 2019
@js-kyle
Copy link
Contributor Author

js-kyle commented Apr 4, 2019

@TheHollidayInn I somehow missed this, I think this is a really good draft, we should try to get this one going again

@TheHollidayInn
Copy link
Contributor

Sounds good! PR posted: #373

@stale
Copy link

stale bot commented Jul 8, 2019

Hello there! 👋
This issue has gone silent. Eerily silent. ⏳
We currently close issues after 100 days of inactivity. It has been 90 days since the last update here.
If needed, you can keep it open by replying here.
Thanks for being a part of the Node.js Best Practices community! 💚

@stale stale bot added the stale label Jul 8, 2019
@stale stale bot closed this as completed Jul 18, 2019
@TheHollidayInn
Copy link
Contributor

Ping as this is being worked on in a PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants