-
Notifications
You must be signed in to change notification settings - Fork 5
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
Initial setup for concurrent workers #46
Conversation
packages/cli/src/api/split.ts
Outdated
.catch((error) => { | ||
console.error(error); | ||
throw error; | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The response is sent before the worker or done with the jobs.
Should wait until the worker are done before sending the response
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the code to wait for the response 🟢
import Parser from "tree-sitter"; | ||
import assert from "assert"; | ||
import { getLanguagePlugin } from "../languagesPlugins"; | ||
import { DepExport } from "../languagesPlugins/types"; | ||
|
||
class SplitRunner { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's delete this class, become kind of useless now. We can do a simple method to replace it.
Maybe in another PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
console.time("Remove errors"); | ||
removeErrors(); | ||
console.timeEnd("Remove errors"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to find a better way to log. Maybe with a fancier logger. Cause here since they now run in parallel, very hard to read the logs. Also probably in another PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type of change
Description
This PR refactors the split functionality to leverage Node.js Worker Threads, enabling concurrent processing. Previously, the splitting operations ran sequentially, which could be time-consuming for larger projects and multiple endpoints. By introducing parallelization with worker threads, we significantly improve the speed and efficiency of the process.
Related Issue
Issue Number: #41
Notes for Reviewer:
@florianbgt We'll not have to worry about thread count and stuff since worker_threads handles everything automatically pretty well and doesn't cause leaks. We can add an extra layer of checking on that but rather to have that in a separate PR to not block this since it's a good improvement already.
Checklist