armeria
armeria copied to clipboard
[WIP] Introduce `CompositeHttpHeadersBase` to replace HttpHeadersUtil.merge()
related issue: https://github.com/line/armeria/issues/5146
Motivation:
https://github.com/line/armeria/blob/3595dacd9e7675fec309a1029a2a97b829c08682/core/src/main/java/com/linecorp/armeria/internal/common/HttpHeadersUtil.java#L46-L46
- On https://github.com/line/armeria/issues/5146, we found that
HttpHeadersUtil.mergeHeaders(..)
occupies a large part of CPU cycles due to expensive deep-copy operations
๐ PRs Overview:
- โ
(this PR) Introduce
CompositeHttpHeadersBase
to replaceHttpHeadersUtil.mergeHeaders(..)
- Introduce
CompositeHttpBeaderBuilder
builder - Introduce
DefaultCompositeRequest, ResponseHeaders
- (finally) Replace
HttpHeadersUtil.mergeHeaders(..)
toComposite*Headers
Modifications:
- Introduce
CompositeHttpHeadersBase
andCompositeStringMultiMap
to replace HttpHeadersUtil.merge()
Result:
- Now we have
CompositeHttpHeadersBase
so can implementDefaultCompositeRequest, ResponseHeaders
๐ Build Scanยฎ (commit: e9fcd38353a5cbfa133bcc3992ca36cf8feb8eb6)
Job name | Status | Build Scanยฎ |
---|---|---|
build-windows-latest-jdk-19 | โ | https://ge.armeria.dev/s/fagki3cgeavso |
build-self-hosted-unsafe-jdk-8 | โ | https://ge.armeria.dev/s/7zm72vtzosdem |
build-self-hosted-unsafe-jdk-19-snapshot-blockhound | โ | https://ge.armeria.dev/s/ry56m32qmgk56 |
build-self-hosted-unsafe-jdk-17-min-java-17-coverage | โ | https://ge.armeria.dev/s/w5ttm4bkqh7jm |
build-self-hosted-unsafe-jdk-17-min-java-11 | โ (failure) | https://ge.armeria.dev/s/qevvxewiy5ptw |
build-self-hosted-unsafe-jdk-17-leak | โ | https://ge.armeria.dev/s/cf22vfdohvxbq |
build-self-hosted-unsafe-jdk-11 | โ | https://ge.armeria.dev/s/cmfbbdk3iuo4e |
build-macos-12-jdk-19 | โ | https://ge.armeria.dev/s/zkj54c54wbns4 |
Codecov Report
Attention: Patch coverage is 84.47552%
with 111 lines
in your changes missing coverage. Please review.
Project coverage is 74.21%. Comparing base (
75d5af1
) to head (e9fcd38
). Report is 194 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #5340 +/- ##
===========================================
+ Coverage 0 74.21% +74.21%
- Complexity 0 21119 +21119
===========================================
Files 0 1803 +1803
Lines 0 77283 +77283
Branches 0 9948 +9948
===========================================
+ Hits 0 57354 +57354
- Misses 0 15254 +15254
- Partials 0 4675 +4675
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Really sorry for late, I'll update this PR within this weekend ๐
Really sorry for late, I think we can reduce complexity of CompositeHeader by supporting only add
with zero-copy.
- If we support zero-copy on
add, set, remove
, logic is too complex and easy to make bug- If we usually don't
set, remove
aftermergeHeaders
, we can minimize complexity ofCompositeHeader
- If we usually don't
I found that spring has similar CompositeAttribute
impl but it has many bugs.
โ
So, I'll check mergeHeaders
's usage and prepare benchmark test too~! ๐