Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Reduce the size of HeaderDescriptor#64105

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

MihaZupan
Copy link
Member

@MihaZupanMihaZupan commentedJan 21, 2022•
edited
Loading

Fixes#62847

NonValidatedAdd_NonValidatedEnumerate:

ToolchainRequestHeadersMeanErrorStdDevMedianRatioRatioSDGen 0Allocated
main2118.64 ns0.274 ns1.300 ns118.28 ns1.000.000.0253256 B
unsafe-pr294.69 ns0.764 ns3.680 ns95.17 ns0.800.030.0222224 B
pr292.44 ns0.269 ns1.303 ns92.51 ns0.780.010.0222224 B
main4210.50 ns0.653 ns3.068 ns208.73 ns1.000.000.0253256 B
unsafe-pr4157.14 ns0.234 ns1.118 ns156.71 ns0.750.010.0222224 B
pr4165.77 ns0.813 ns3.949 ns165.00 ns0.790.030.0222224 B
main6356.20 ns0.488 ns2.366 ns354.75 ns1.000.000.0467472 B
unsafe-pr6279.82 ns0.794 ns3.753 ns277.51 ns0.790.010.0372376 B
pr6294.23 ns0.400 ns1.970 ns293.46 ns0.830.010.0372376 B
main8484.50 ns0.629 ns3.003 ns482.65 ns1.000.000.0467472 B
unsafe-pr8347.89 ns1.147 ns5.490 ns344.33 ns0.720.010.0372376 B
pr8355.82 ns1.256 ns6.072 ns353.78 ns0.730.020.0372376 B
main12903.92 ns1.075 ns5.127 ns907.11 ns1.000.000.0868880 B
unsafe-pr12732.21 ns0.769 ns3.643 ns731.08 ns0.810.000.0648656 B
pr12779.44 ns2.077 ns10.180 ns774.05 ns0.860.020.0648656 B
main161,358.70 ns0.430 ns2.032 ns1,358.55 ns1.000.000.0858880 B
unsafe-pr161,170.03 ns7.485 ns39.010 ns1,164.91 ns0.860.030.0648656 B
pr161,259.05 ns5.470 ns28.508 ns1,241.47 ns0.930.020.0648656 B
main323,889.28 ns17.269 ns90.002 ns3,887.74 ns1.000.000.16401,672 B
unsafe-pr323,580.34 ns13.858 ns72.100 ns3,570.95 ns0.920.030.11831,192 B
pr324,046.49 ns23.530 ns122.632 ns4,030.22 ns1.040.030.11831,192 B

SendAsync (Create and serialize the request, parse the response and enumerate 4 response headers):

ToolchainRequestHeadersMeanErrorStdDevMedianRatioRatioSDGen 0Allocated
main41.644 μs0.0062 μs0.0311 μs1.630 μs1.000.000.0935944 B
original-pr41.639 μs0.0057 μs0.0281 μs1.630 μs1.000.030.0858880 B
pr41.650 μs0.0038 μs0.0191 μs1.653 μs1.000.020.0858880 B
main82.084 μs0.0013 μs0.0064 μs2.082 μs1.000.000.11441,160 B
original-pr82.055 μs0.0030 μs0.0145 μs2.055 μs0.990.010.09921,032 B
pr82.026 μs0.0068 μs0.0327 μs2.028 μs0.970.020.09921,032 B
main163.428 μs0.0039 μs0.0185 μs3.424 μs1.000.000.15261,568 B
original-pr163.328 μs0.0044 μs0.0209 μs3.324 μs0.970.000.12971,312 B
pr163.419 μs0.0141 μs0.0675 μs3.446 μs1.000.020.12971,312 B

@MihaZupanMihaZupan added this to the7.0.0 milestoneJan 21, 2022
@MihaZupanMihaZupan requested review fromgeoffkizer anda teamJanuary 21, 2022 17:46
@ghost
Copy link

Tagging subscribers to this area: @dotnet/ncl
See info inarea-owners.md if you want to be subscribed.

Issue Details

Fixes#62847

NonValidatedAdd_NonValidatedEnumerate:

ToolchainRequestHeadersMeanErrorStdDevMedianRatioRatioSDGen 0Allocated
main2160.2 ns0.29 ns1.43 ns160.0 ns1.000.000.0610256 B
pr2124.0 ns0.25 ns1.25 ns124.1 ns0.770.010.0534224 B
main4324.5 ns0.84 ns4.22 ns324.3 ns1.000.000.0610256 B
pr4251.3 ns0.62 ns3.15 ns252.2 ns0.770.020.0534224 B
main6595.2 ns8.47 ns42.60 ns567.3 ns1.000.000.1125472 B
pr6488.1 ns1.17 ns5.87 ns490.8 ns0.820.050.0896376 B
main8724.4 ns1.48 ns7.46 ns721.8 ns1.000.000.1125472 B
pr8614.1 ns0.73 ns3.66 ns613.3 ns0.850.010.0896376 B
main121,297.7 ns2.04 ns10.36 ns1,299.7 ns1.000.000.2098880 B
pr121,173.3 ns3.95 ns19.66 ns1,184.3 ns0.900.020.1564656 B
main161,864.4 ns1.80 ns9.16 ns1,862.6 ns1.000.000.2098880 B
pr161,782.5 ns11.47 ns57.91 ns1,811.9 ns0.960.030.1564656 B
main325,049.9 ns5.34 ns27.48 ns5,045.2 ns1.000.000.39671,672 B
pr324,895.6 ns24.95 ns128.69 ns4,836.3 ns0.970.030.28231,192 B

SendAsync (Create and serialize the request, parse the response and enumerate 4 response headers)

ToolchainRequestHeadersMeanErrorStdDevMedianRatioGen 0Allocated
main42.793 us0.0035 us0.0177 us2.789 us1.000.2251944 B
pr42.765 us0.0030 us0.0147 us2.767 us0.990.2098880 B
main83.469 us0.0020 us0.0099 us3.469 us1.000.27471,160 B
pr83.431 us0.0072 us0.0359 us3.415 us0.990.24411,032 B
main165.137 us0.0048 us0.0243 us5.139 us1.000.37381,568 B
pr165.120 us0.0062 us0.0314 us5.113 us1.000.31281,312 B
Author:MihaZupan
Assignees:-
Labels:

area-System.Net.Http

Milestone:7.0.0

@ghostghost assignedMihaZupanJan 21, 2022
@@ -96,7 +96,7 @@ internal static class QPackStaticTable
(new HeaderDescriptor(KnownHeaders.Authorization), ""), // 84
(new HeaderDescriptor(KnownHeaders.ContentSecurityPolicy), "script-src 'none'; object-src 'none'; base-uri 'none'"), // 85
(new HeaderDescriptor("early-data"), "1"), // 86
(new HeaderDescriptor("expect-ct"), ""), // 87
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

This is an existing bug in HTTP/3 wherex-xss-protection andexpect-ct lookups/removes would silently fail if added viaOnStaticIndexedHeader.

public HttpHeaderParser? Parser => (_descriptor as KnownHeader)?.Parser;
public HttpHeaderType HeaderType => _descriptor is KnownHeader knownHeader ? knownHeader.HeaderType : HttpHeaderType.Custom;

public bool Equals(KnownHeader other) => ReferenceEquals(_descriptor, other);
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a little weird to call this method "Equals".

I'd suggest calling it "IsKnownHeader(KnownHeader header)" but we already have a method called that.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Would you find the== operator more fitting here?

@MihaZupan
Copy link
MemberAuthor

@dotnet/ncl Please take a look

Copy link
Member

@ManickaPManickaP left a comment

Choose a reason for hiding this comment

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

LGTM thanks

@@ -96,7 +96,7 @@ internal static class QPackStaticTable
(new HeaderDescriptor(KnownHeaders.Authorization), ""), // 84
(new HeaderDescriptor(KnownHeaders.ContentSecurityPolicy), "script-src 'none'; object-src 'none'; base-uri 'none'"), // 85
(new HeaderDescriptor("early-data"), "1"), // 86
(new HeaderDescriptor("expect-ct"), ""), // 87
(new HeaderDescriptor(KnownHeaders.ExpectCT), ""), // 87
Copy link
Member

Choose a reason for hiding this comment

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

Should we be adding KnownHeaders entries for the othernew HeaderDescriptor(string) entries in this table?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

That would be my hope as part of#63750, I updated the post to includeQPackStaticTable

@MihaZupanMihaZupan merged commit4207296 intodotnet:mainFeb 1, 2022
@ghostghost locked asresolvedand limited conversation to collaboratorsMar 3, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@danmoseleydanmoseleydanmoseley left review comments

@geoffkizergeoffkizergeoffkizer left review comments

@stephentoubstephentoubstephentoub approved these changes

@ManickaPManickaPManickaP approved these changes

Assignees

@MihaZupanMihaZupan

Projects
None yet
Milestone
7.0.0
Development

Successfully merging this pull request may close these issues.

Consider reducing HeaderDescriptor size by using object field
5 participants
@MihaZupan@stephentoub@danmoseley@ManickaP@geoffkizer
[8]ページ先頭
©2009-2025 Movatter.jp