refit icon indicating copy to clipboard operation
refit copied to clipboard

Published 20 hours ago • verified publisher icon reactiveui

feat: WIP optimize url creation

Open TimothyMakkison opened this issue 1 year ago • 1 comments

WIP; Uses the first call to determine the substitutions for the URL.

  • This might be a "breaking change" as the url conversion is done later on and in bulk, in addition to calling UrlParameterFormatter.Format on the same value if it appears several times in the URL.
    • If this is an issue I can use a ValueListBuilder to preserve the order of operations and to cache values,
  • ParameterFragment is my messy attempt to create a tagged union.
    • Instead of having a string Value property it could be an index and count representing the reevant area in the original url.
  • Uses ValueStringBuilder I'll need to rebase this after #1719 is merged.
  • This would ideally be done in the source generator.
  • Could do with some benchmarks, especially for startup times.

TimothyMakkison avatar Jun 22 '24 22:06 TimothyMakkison

Codecov Report

Attention: Patch coverage is 91.72414% with 12 lines in your changes missing coverage. Please review.

Project coverage is 83.54%. Comparing base (6ebeda5) to head (94bf0a5). Report is 59 commits behind head on main.

Files Patch % Lines
Refit/RestMethodInfo.cs 94.11% 3 Missing and 2 partials :warning:
Refit/RequestBuilderImplementation.cs 92.98% 2 Missing and 2 partials :warning:
Refit/UnreachableException.cs 0.00% 3 Missing :warning:
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1730      +/-   ##
==========================================
- Coverage   87.73%   83.54%   -4.20%     
==========================================
  Files          33       37       +4     
  Lines        2348     2455     +107     
  Branches      294      354      +60     
==========================================
- Hits         2060     2051       -9     
- Misses        208      319     +111     
- Partials       80       85       +5

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jun 22 '24 22:06 codecov[bot]

Rebased and done some modifications. Added Refit.UnreachableException for cases that should not be able to happen.

Not sure what I'm missing but bizarrely the benchmarks don't show a major change in memory usage. I'll review this a different day but I might abandon this and reuse the logic for the source generator.

Before

Method Mean Error StdDev Gen0 Gen1 Allocated
ConstantRouteAsync 2.045 us 0.0395 us 0.0638 us 0.6828 0.0114 6.28 KB
DynamicRouteAsync 2.601 us 0.0518 us 0.0532 us 0.7172 0.0038 6.6 KB
ComplexDynamicRouteAsync 3.799 us 0.0651 us 0.0891 us 0.7858 - 7.24 KB
ObjectRequestAsync 4.560 us 0.0886 us 0.1021 us 0.8698 - 8.05 KB
ComplexRequestAsync 12.951 us 0.2575 us 0.4643 us 1.5259 - 14.19 KB

Changes

Method Mean Error StdDev Median Gen0 Gen1 Allocated
ConstantRouteAsync 3.037 us 0.0602 us 0.1174 us 3.028 us 0.6828 - 6.3 KB
DynamicRouteAsync 3.195 us 0.2061 us 0.6076 us 3.410 us 0.7172 0.0076 6.63 KB
ComplexDynamicRouteAsync 3.054 us 0.0496 us 0.0464 us 3.033 us 0.7858 0.0076 7.23 KB
ObjectRequestAsync 4.194 us 0.0613 us 0.0512 us 4.180 us 0.8698 - 8.04 KB
ComplexRequestAsync 11.623 us 0.1973 us 0.1749 us 11.590 us 1.5259 0.0153 14.16 KB

TimothyMakkison avatar Jul 06 '24 19:07 TimothyMakkison

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

github-actions[bot] avatar Aug 05 '24 00:08 github-actions[bot]