Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Vectorize ValidateHeaderCharacters #1301

Closed
wants to merge 2 commits into from
Closed

Vectorize ValidateHeaderCharacters #1301

wants to merge 2 commits into from

Conversation

benaadams
Copy link
Contributor

@benaadams benaadams commented Jan 10, 2017 •
edited
Loading

x 1.9 times faster for plaintext (13 + text/plain)
x 2.5 - 3.5 times faster for longer headers like Content-Security-Policy or large cookies

Newer version of part of #1000 "More Vector performance" for ValidateHeaderCharacters

        Method |                               Value |          Mean |     StdDev |            RPS |
-------------- |------------------------------------ |-------------- |----------- |--------------- |
  ValidateIter |                                  13 |     4.7229 ns |  0.0492 ns | 211,733,373.12 |
ValidateVector |                                  13 |     5.0420 ns |  0.0366 ns | 198,332,954.88 |
  ValidateIter |                          text/plain |    16.6734 ns |  0.3520 ns |  59,975,946.30 |
ValidateVector |                          text/plain |     6.2288 ns |  0.0638 ns | 160,543,964.49 |
  ValidateIter |             Content-Security-Policy |    37.6319 ns |  0.2950 ns |  26,573,199.60 |
ValidateVector |             Content-Security-Policy |    10.7913 ns |  0.1514 ns |  92,667,513.70 |
  ValidateIter | Mozilla/5.0 (Windows... 109 byte UA |   115.1069 ns |  3.0876 ns |   8,687,577.53 |
ValidateVector | Mozilla/5.0 (Windows... 109 byte UA |    36.4889 ns |  0.3958 ns |  27,405,596.46 |
  ValidateIter | ZGVmYXVsdC1zcmM... 1168 byte cookie | 1,047.3965 ns | 10.6608 ns |     954,748.25 |
ValidateVector | ZGVmYXVsdC1zcmM... 1168 byte cookie |   362.3385 ns |  2.9977 ns |   2,759,850.40 |

Testing at https://github.com/benaadams/PerfTesting/tree/master/src/HeaderValidation

/cc @halter73

@benaadams benaadams mentioned this pull request Jan 10, 2017
Closed
benaadams
benaadams commented Jan 10, 2017
View reviewed changes
src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameHeaders.cs
ThrowInvalidHeaderCharacter(ch);
var stringUint = (uint*)(pHeader + offset);
offset += sizeof(uint);
if (((*stringUint | (*stringUint + 0x00010001u) | (*stringUint - 0x00200020u)) & 0xff80ff80u) != 0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't need to *stringUint | as (*stringUint - 0x00200020u) should pickup the high sets

@benaadams
Copy link
Contributor Author

Updated with benchmarks (Vector-size 16, will be different for vector 32/64)

richardszalay
richardszalay reviewed Jan 11, 2017
View reviewed changes
src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameHeaders.cs
{
var vSub = Vector.AsVectorUInt16(new Vector<uint>(0x00200020u));
// 0x7e as highest ascii (don't include DEL)
// 0x20 as lowerest ascii (space)
Copy link

Choose a reason for hiding this comment

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

lowerest?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed :)

@benaadams
Copy link
Contributor Author

Rebased

@benaadams
Copy link
Contributor Author

benaadams commented Jan 23, 2017 •
edited
Loading

Better testing using benchmark from #1308

Before

        Method |                 Type |          Mean |        Median |           RPS |
-------------- |--------------------- |-------------- |-------------- |-------------- |
    SetHeaders |               Common | 1,435.1032 ns | 1,440.4368 ns |    696,814.01 |
    SetHeaders |            Plaintext |   154.9097 ns |   154.8979 ns |  6,455,373.13 |
    SetHeaders |              Unknown | 2,421.2750 ns | 2,427.1806 ns |    413,005.54 |

After

        Method |                 Type |          Mean |        Median |           RPS |
-------------- |--------------------- |-------------- |-------------- |-------------- |
    SetHeaders |               Common |   952.3173 ns |   955.1426 ns |  1,050,070.16 |
    SetHeaders |            Plaintext |   146.2217 ns |   146.1196 ns |  6,838,927.89 |
    SetHeaders |              Unknown | 1,694.4606 ns | 1,764.4501 ns |    590,158.32 |

@benaadams benaadams mentioned this pull request Jan 23, 2017
Closed
@davidfowl davidfowl closed this Feb 19, 2017
@benaadams benaadams deleted the vector-perf branch March 27, 2018 05:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@richardszalay richardszalay richardszalay left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@benaadams @richardszalay @davidfowl @dnfclas