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

perf(file_server): hoistStatic and simplify operations #3

Closed
wants to merge 5 commits into from

Conversation

markthree
Copy link

@markthree markthree commented Jul 4, 2023

The file_server example has been optimized without unfairness。

Here is the performance of file_server/deno_server.ts before and after optimization

Before optimization

deno_serve_before


After optimization

deno_serve_after

Other examples have been more or less optimized.

Test data provided by plow

Similar optimization tips in the community:

@Im-Beast
Copy link
Owner

Im-Beast commented Jul 4, 2023

I'm skeptical about this PR for multiple reasons. It might improve performance but gives up on readability, and it isn't what most people would do – std manual shows usage as shown below:
image

Additionally by running benchmarks on my PC it seems like it doesn't do anything regarding performance (if not degrades it). Benchmark results from your code show some receiving 0.5-3% decline in throughput. These amounts (looking that the second place also dropped by the same amount) mean that these changes do neglibile amounts of gains while loose substantial amounts of readability.

[BEFORE]
image
[AFTER]
image

I'm still open for these kinds of PR's. If these results show significant enough performance gains on anyone else's computers, please let me know.

@markthree
Copy link
Author

I'm sorry I only tested file_server/deno_server.ts and used plow to keep hitting http://localhost:8000/doggy.jpeg
Optimized for better performance on the computer it is on. Of course I have no more other computers to provide bench data.
I will close this pr for the balance of readability and performance improvement.
Thank you for providing the performance bench for all cases.

@markthree markthree closed this Jul 5, 2023
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.

None yet

2 participants