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 of directory listings #357

Merged
merged 5 commits into from
Apr 24, 2024

Conversation

palant
Copy link
Contributor

@palant palant commented Apr 22, 2024

Description

The benchmarks in #355 indicated that SWS directory listings are slow, so I looked into how they can be improved. I managed to improve performance of the directory listing benchmark by around 10% which is ok but not really sufficient to catch up with the competition.

The performance improvements are largely due to the way the listing is constructed. There is a large improvement by not percent-decoding the file name but using the name we already have. And constructing table_rows variable from an iterator also improves things.

Remarkably, the changes to sort_file_entries() function are barely noticeable in the benchmark. Presumably, Rust optimizer already figured out that no sorting needs to be performed for order_code == 6 and that variables like name don’t need to be overwritten with the same value repeatedly.

How Has This Been Tested?

I did functional testing on Linux.

Copy link

semanticdiff-com bot commented Apr 22, 2024

Review changes with SemanticDiff.

Analyzed 1 of 1 files.

Overall, the semantic diff is 33% smaller than the GitHub diff.

Filename Status
✔️ src/directory_listing.rs 32.82% smaller

@joseluisq joseluisq added enhancement New feature or request v2 v2 release performance Related to server performance labels Apr 22, 2024
@palant
Copy link
Contributor Author

palant commented Apr 23, 2024

For reference, I did try using write!() into a string to compose the entire page and not merely the table. This made the code easier to follow in some respects but it didn’t have any noteworthy impact on the performance. So it seems that there are no significant gains left in the string composition any more. Yes, I should probably try an actual profiler.

@joseluisq
Copy link
Collaborator

Looking at the benchmark results, it seems like there is a slightly small improvement when listing large directory files but for small directories is even slightly slower. I did compare the current master results with the PR.

Copy link
Collaborator

@joseluisq joseluisq left a comment

Choose a reason for hiding this comment

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

I left some minor comments to check first.
But overall for me looks ok.

src/directory_listing.rs Show resolved Hide resolved
src/directory_listing.rs Show resolved Hide resolved
src/directory_listing.rs Show resolved Hide resolved
@palant
Copy link
Contributor Author

palant commented Apr 24, 2024

Looking at the benchmark results, it seems like there is a slightly small improvement when listing large directory files but for small directories is even slightly slower.

The benchmark results aren’t stable enough to measure small time differences reliably. You have to consider the other web servers as the baseline – their time also went up, so it’s something related to the action runner, not the code. If I check the proportion for SWS time and other servers on the 50th percentile (median), for an empty directory it went down consistently. Not a huge change but a few percent. In fact, the results are actually less conclusive for large directories.

@palant
Copy link
Contributor Author

palant commented Apr 24, 2024

Looking at the benchmark results, it seems like there is a slightly small improvement when listing large directory files but for small directories is even slightly slower.

The benchmark results aren’t stable enough to measure small time differences reliably. You have to consider the other web servers as the baseline – their time also went up, so it’s something related to the action runner, not the code. If I check the proportion for SWS time and other servers on the 50th percentile (median), for an empty directory it went down consistently. Not a huge change but a few percent. In fact, the results are actually less conclusive for large directories.

Looking at the new test run – yes, the performance changes here are negligible. Somehow that looked differently in my previous benchmark runs.

@palant
Copy link
Contributor Author

palant commented Apr 24, 2024

Looking at the benchmark results, it seems like there is a slightly small improvement when listing large directory files but for small directories is even slightly slower.

The benchmark results aren’t stable enough to measure small time differences reliably. You have to consider the other web servers as the baseline – their time also went up, so it’s something related to the action runner, not the code. If I check the proportion for SWS time and other servers on the 50th percentile (median), for an empty directory it went down consistently. Not a huge change but a few percent. In fact, the results are actually less conclusive for large directories.

I’ve done some benchmarking on my computer, the numbers are more stable then. I could confirm – the performance for a large directory does improve but it is only around 3%. Not overwhelming but measurable.

Copy link
Collaborator

@joseluisq joseluisq left a comment

Choose a reason for hiding this comment

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

Nice! Dir listing is now way faster than before.
https://github.com/static-web-server/static-web-server/actions/runs/8823589340

Awesome!

@joseluisq joseluisq merged commit cfd1390 into static-web-server:master Apr 24, 2024
34 checks passed
@palant palant deleted the faster-dir-listing branch April 24, 2024 22:08
@joseluisq joseluisq added this to the v2.30.0 milestone Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request performance Related to server performance v2 v2 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants