Skip to content
This repository has been 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

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

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.

None yet
4 participants
@benaadams @richardszalay @davidfowl @dnfclas