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

Improve performance (throughput) #4

Merged
merged 1 commit into from
Feb 21, 2022
Merged

Conversation

lambdalisue
Copy link
Contributor

@lambdalisue lambdalisue commented Feb 20, 2022

Personally, I feel that the complexity of a few hundred milliseconds for 200,000 entries is not worth the cost. Therefore, I will leave it to you to decide whether to merge them.

When using async iterator, there seems to be a problem that iterating on individual elements affects performance.
In other words, it's likely that Deno has the same problem as the following issue that exists in Node.

nodejs/node#31979

So I tried to avoid this problem by using AsyncGenerator<T[]> instead and seems working.

Results of {'profile': 1}

I've made 200,000 files with

#!/bin/bash
mkdir -p out
for i in {1..200000}; do
  touch out/$i.txt
done

Then run call ddu#start({'profile': 1}) on out directory.

Before #3 After #3 This PR
1167 ms 1591 ms 1213 ms
1217 ms 1569 ms 1165 ms
1165 ms 1588 ms 1148 ms
1186 ms 1562 ms 1155 ms
1121 ms 1493 ms 1151 ms

It's likely that Deno has the same problem as the following issue that
exists in Node.

nodejs/node#31979

So we tried to avoid this problem by using `AsyncGenerator<T[]>`
instead.
@Shougo
Copy link
Owner

Shougo commented Feb 21, 2022

Hi, I have tested the branch. It seems 5%~10% faster than main. But it is not so huge difference...

@Shougo
Copy link
Owner

Shougo commented Feb 21, 2022

I will merge it. I think it is better than previous version and abortable merge is needed.
Thanks.

@Shougo Shougo merged commit e82acd9 into Shougo:main Feb 21, 2022
@lambdalisue lambdalisue deleted the fix-performance branch February 21, 2022 03:59
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