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

Refactor request configuration internal representation #292

Merged
merged 14 commits into from
Feb 20, 2021

Conversation

sagebind
Copy link
Owner

Refactor how request configuration is represented internally from a purely extension-based model into a centralized struct. This reduces the generalized code somewhat, but makes it easier to understand for people less familiar with the codebase.

More importantly, this delivers a 5-10% performance improvement in synthetic benchmarks, since it greatly reduces the number of wasteful hashmap lookups we do when trying to apply request configuration to an easy handle.

@sagebind sagebind added the performance An enhancement or problem with performance label Jan 14, 2021
@codecov
Copy link

codecov bot commented Jan 15, 2021

Codecov Report

Merging #292 (7d0d18c) into master (e0b6fd8) will increase coverage by 2.48%.
The diff coverage is 76.92%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #292      +/-   ##
==========================================
+ Coverage   73.02%   75.51%   +2.48%     
==========================================
  Files          53       55       +2     
  Lines        2773     2822      +49     
==========================================
+ Hits         2025     2131     +106     
+ Misses        748      691      -57     
Impacted Files Coverage Δ
src/auth.rs 67.18% <ø> (ø)
src/config/proxy.rs 100.00% <ø> (ø)
src/config/redirect.rs 100.00% <ø> (ø)
src/response.rs 58.33% <ø> (ø)
src/redirect.rs 77.77% <40.00%> (+0.63%) ⬆️
src/config/client.rs 62.50% <62.50%> (ø)
src/client.rs 73.09% <65.51%> (+5.76%) ⬆️
src/config/mod.rs 72.03% <76.92%> (+15.36%) ⬆️
src/request.rs 82.60% <76.92%> (+54.60%) ⬆️
src/config/request.rs 77.94% <77.94%> (ø)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e0b6fd8...7d0d18c. Read the comment docs.

@sagebind sagebind marked this pull request as ready for review February 20, 2021 05:46
@sagebind
Copy link
Owner Author

@teto-bot rustfmt

@sagebind sagebind merged commit dc5d0ee into master Feb 20, 2021
@sagebind sagebind deleted the config-consolidation branch February 20, 2021 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance An enhancement or problem with performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants