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

Refactor throttling logic for async function #668

Merged
merged 6 commits into from
Oct 30, 2024

Conversation

kmagiera
Copy link
Member

When testing Radon I encountered an issue with newly cloned repo in which the IDE would generate an excessive number of npm subprocesses. The issue turned out to be related to fingerprint library, which on some occasions can spawn at least three npm processes. The main problem though was that this problem was amplified by the fact we'd request fingerprint calculation upon the watched files update. Even though the requests were throttled, because they'd take significant time to compute, we'd end up spawning many concurrent processes. This was specifically noticeable at the moment when the project was installing node modules which generates a ton of fs events that resulted in the fingerprint calculation getting triggered.

The solution this PR is implementing is to add a new throttling mechanism for async functions. It is a non-standard mechanism hence we're introducing a new method. The new mechanism is then used to throttle async code that calculates the fingerprint.

The mechanism works by keeping the following constraints:

  1. it never allows for the method to be called more often than every X millisecond
  2. it never allows for the method to be executed concurrently
  3. when called multiple times, it is guaranteed that the latest call with the most recent arguments will be executed

How Has This Been Tested:

  1. Clone some open source RN project
  2. Without installing node modules just open it with the IDE
  3. On my laptop before this change, I'd observe an excessive number of npm processes being spawned.

Copy link

vercel bot commented Oct 28, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
radon-ide ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 30, 2024 1:31pm

Copy link
Collaborator

@filip131311 filip131311 left a comment

Choose a reason for hiding this comment

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

Left some inline comments

…ents case, also don't use timeout variable that got copied from basic throttle implementation
Copy link
Collaborator

@filip131311 filip131311 left a comment

Choose a reason for hiding this comment

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

LGTM

@kmagiera kmagiera merged commit 0084c45 into main Oct 30, 2024
4 checks passed
@kmagiera kmagiera deleted the kmagiera/async-throttle-fingerprint branch October 30, 2024 16:37
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

Successfully merging this pull request may close these issues.

2 participants