Skip to content

Conversation

rokonec
Copy link
Member

@rokonec rokonec commented Nov 30, 2020

Optimized to use 8 bits lookup tables tree.

dotnet/runtime#1506

Shared with runtime. Already reviewed by @Tratcher in PR dotnet/runtime#43603

Fixes #18943

@rokonec rokonec requested a review from a team November 30, 2020 09:14
@BrennanConroy
Copy link
Member

Bot opened the same PR #28242

@Tratcher
Copy link
Member

I thought you were going to send this PR and run the benchmarks before merging the change in runtime? Do we still have benchmarks we need to run here?

I'll close #28242 since it's redundant.

@Tratcher Tratcher self-assigned this Nov 30, 2020
@rokonec
Copy link
Member Author

rokonec commented Nov 30, 2020

@Tratcher benchmarks has been created and run in https://github.com/dotnet/runtime/blob/master/src/libraries/System.Net.Http/tests/PerformanceTests/HPackHuffmanBenchmark/Program.cs

Latest version of comparison benchmarked had following results:

Method Mean Error StdDev Op/s Ratio
Old 2,146.14 ns 40.559 ns 39.835 ns 465,953.3 1.00
New 537.22 ns 2.607 ns 2.439 ns 1,861,447.5 0.25

Unfortunately


do not contain benchmarks for huffman encoded headers.

Shall I add it there?

@Tratcher
Copy link
Member

Ok. I don't know if we need duplication of those benchmarks. @halter73 do you care?

@halter73
Copy link
Member

I'm fine with not duplicating. Maybe we should remove the HPACK microbenchmarks from Kestrel if they're covered by the runtime so we don't waste time on the Kestrel versions.

@Tratcher Tratcher merged commit 7ab0d78 into dotnet:master Nov 30, 2020
@Tratcher Tratcher added this to the 6.0-preview1 milestone Nov 30, 2020
@rokonec rokonec deleted the rokonec/runtime-1506-optimize-huffman-decoding-using-lut branch November 30, 2020 21:31
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.

The runtime<->aspnetcore shared src is out of sync
4 participants