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

Remove the concurrency option #434

Merged
merged 1 commit into from
Dec 25, 2023
Merged

Remove the concurrency option #434

merged 1 commit into from
Dec 25, 2023

Conversation

mrmlnc
Copy link
Owner

@mrmlnc mrmlnc commented Dec 24, 2023

What is the purpose of this pull request?

In most cases, users do not use this option. There are only a couple of places on GitHub right now where users specify concurrency. The value passed there is os.cpu().length.

When specifying any value, the concurrency is always limited by the libuv library.

In Node, there are [two types of threads][nodejs_thread_pool]: Event Loop (code) and a Thread Pool (fs, dns, …). The thread pool size controlled by the UV_THREADPOOL_SIZE environment variable. Its default size is 4 ([documentation][libuv_thread_pool]). The pool is one for all tasks within a single Node process.

Any code can make 4 real concurrent accesses to the file system. The rest of the FS requests will wait in the queue.

📖 Each new instance of FG in the same Node process will use the same Thread pool.

But this package also has the concurrency option. This option allows you to control the number of concurrent accesses to the FS at the package level. By default, this package has a value equal to the number of cores available for the current Node process. This allows you to set a value smaller than the pool size (concurrency: 1) or, conversely, to prepare tasks for the pool queue more quickly (concurrency: Number.POSITIVE_INFINITY).

So, in fact, this package can only make 4 concurrent requests to the FS. You can increase this value by using an environment variable (UV_THREADPOOL_SIZE), but in practice this does not give a multiple advantage.

Currently, the concurrency option is not functioning correctly. For each task, a reader with its own queue is created. This means that if you specify the concurrency:1 option and define three tasks, three calls will actually be sent to libuv thread pool instead of one.

fast-glob/src/index.ts

Lines 27 to 34 in 4b71e29

const settings = new Settings(options);
const reader = new ReaderAsync(settings);
const provider = new ProviderAsync(reader, settings);
const tasks = getTasks(source, settings);
const promises = tasks.map((task) => provider.read(task));
const result = await Promise.all(promises);

Currently, it would be easier for me to simply remove this option rather than create a shared queue for all tasks.

If anyone needs this opportunity, please let me know.

What changes did you make? (Give an overview)

@mrmlnc mrmlnc added this to the 4.0.0 milestone Dec 24, 2023
@mrmlnc mrmlnc marked this pull request as ready for review December 25, 2023 08:02
@mrmlnc mrmlnc merged commit a1a07fa into master Dec 25, 2023
19 checks passed
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.

1 participant