Use perfect-hash in HttpUriScheme.GetKnownMethod(string) instead of trie

[gfoidl]

Description

In HttpUtilities.GetKnownMethod(string value) a trie based approach is used.

Recently I stumbled accross Fast parsing HTTP verbs from Wojciech Muła and gave it a try.
The approach uses gperf -- Perfect Hash Function Generator to generate a perfect hash function which computes / looks up a key which can be used to get the known HTTP-method.

Benchmark results

|  Method |  Method |     Mean | Ratio |
|-------- |-------- |---------:|------:|
| Default | CONNECT | 4.788 ns |  1.00 |
|      PR | CONNECT | 1.889 ns |  0.39 |
|         |         |          |       |
| Default |  DELETE | 4.293 ns |  1.00 |
|      PR |  DELETE | 2.009 ns |  0.47 |
|         |         |          |       |
| Default |     GET | 3.526 ns |  1.00 |
|      PR |     GET | 1.935 ns |  0.55 |
|         |         |          |       |
| Default |    HEAD | 3.983 ns |  1.00 |
|      PR |    HEAD | 1.979 ns |  0.51 |
|         |         |          |       |
| Default | OPTIONS | 4.789 ns |  1.00 |
|      PR | OPTIONS | 1.960 ns |  0.40 |
|         |         |          |       |
| Default |   PATCH | 4.778 ns |  1.00 |
|      PR |   PATCH | 2.206 ns |  0.46 |
|         |         |          |       |
| Default |    POST | 4.584 ns |  1.00 |
|      PR |    POST | 1.873 ns |  0.41 |
|         |         |          |       |
| Default |     PRI | 7.030 ns |  1.00 |
|      PR |     PRI | 4.799 ns |  0.68 |
|         |         |          |       |
| Default |     PUT | 4.156 ns |  1.00 |
|      PR |     PUT | 1.916 ns |  0.46 |
|         |         |          |       |
| Default |   TRACE | 4.372 ns |  1.00 |
|      PR |   TRACE | 1.951 ns |  0.45 |

(I ran the benchmark several times to verify that the numbers are correct. I didn't believe in the first runs 😉 ... it's less code, traded with lookups)

Additional notes for gperf

As the lookup table isn't intuitive here the steps needed to create it.

Version of gperf: 3.1

File knownverbs.txt:

CONNECT
DELETE
GET
HEAD
OPTIONS
PATCH
POST
PUT
TRACE

Command to run the generator:

gperf --output-file=knownverbs.c knownverbs.txt

This generator spits out C-code, which got ported to C# afterwards.

Note: there are plenty more tools for creating perfect-hashes, but this one is by far the easiest to use.

Question: gperf is GNU General Public License. Here only the output of that tool is used. To my understanding using this here doesn't violate GPL, but please confirm.

Generated C-code

/* ANSI-C code produced by gperf version 3.1 */
/* Command-line: gperf --output-file=knownverbs.c knownverbs.txt  */
/* Computed positions: -k'1' */

#if !((' ' == 32) && ('!' == 33) && ('"' == 34) && ('#' == 35) \
      && ('%' == 37) && ('&' == 38) && ('\'' == 39) && ('(' == 40) \
      && (')' == 41) && ('*' == 42) && ('+' == 43) && (',' == 44) \
      && ('-' == 45) && ('.' == 46) && ('/' == 47) && ('0' == 48) \
      && ('1' == 49) && ('2' == 50) && ('3' == 51) && ('4' == 52) \
      && ('5' == 53) && ('6' == 54) && ('7' == 55) && ('8' == 56) \
      && ('9' == 57) && (':' == 58) && (';' == 59) && ('<' == 60) \
      && ('=' == 61) && ('>' == 62) && ('?' == 63) && ('A' == 65) \
      && ('B' == 66) && ('C' == 67) && ('D' == 68) && ('E' == 69) \
      && ('F' == 70) && ('G' == 71) && ('H' == 72) && ('I' == 73) \
      && ('J' == 74) && ('K' == 75) && ('L' == 76) && ('M' == 77) \
      && ('N' == 78) && ('O' == 79) && ('P' == 80) && ('Q' == 81) \
      && ('R' == 82) && ('S' == 83) && ('T' == 84) && ('U' == 85) \
      && ('V' == 86) && ('W' == 87) && ('X' == 88) && ('Y' == 89) \
      && ('Z' == 90) && ('[' == 91) && ('\\' == 92) && (']' == 93) \
      && ('^' == 94) && ('_' == 95) && ('a' == 97) && ('b' == 98) \
      && ('c' == 99) && ('d' == 100) && ('e' == 101) && ('f' == 102) \
      && ('g' == 103) && ('h' == 104) && ('i' == 105) && ('j' == 106) \
      && ('k' == 107) && ('l' == 108) && ('m' == 109) && ('n' == 110) \
      && ('o' == 111) && ('p' == 112) && ('q' == 113) && ('r' == 114) \
      && ('s' == 115) && ('t' == 116) && ('u' == 117) && ('v' == 118) \
      && ('w' == 119) && ('x' == 120) && ('y' == 121) && ('z' == 122) \
      && ('{' == 123) && ('|' == 124) && ('}' == 125) && ('~' == 126))
/* The character set is not based on ISO-646.  */
#error "gperf generated tables don't work with this execution character set. Please report a bug to <[email protected]>."
#endif


#define TOTAL_KEYWORDS 9
#define MIN_WORD_LENGTH 3
#define MAX_WORD_LENGTH 7
#define MIN_HASH_VALUE 3
#define MAX_HASH_VALUE 12
/* maximum key range = 10, duplicates = 0 */

#ifdef __GNUC__
__inline
#else
#ifdef __cplusplus
inline
#endif
#endif
static unsigned int
hash (register const char *str, register size_t len)
{
  static unsigned char asso_values[] =
    {
      13, 13, 13, 13, 13, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13, 13,  5,  0, 13,
      13,  0,  0, 13, 13, 13, 13, 13, 13,  0,
       5, 13, 13, 13,  0, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13, 13, 13, 13, 13,
      13, 13, 13, 13, 13, 13
    };
  return len + asso_values[(unsigned char)str[0]];
}

const char *
in_word_set (register const char *str, register size_t len)
{
  static const char * wordlist[] =
    {
      "", "", "",
      "GET",
      "HEAD",
      "TRACE",
      "DELETE",
      "OPTIONS",
      "PUT",
      "POST",
      "PATCH",
      "",
      "CONNECT"
    };

  if (len <= MAX_WORD_LENGTH && len >= MIN_WORD_LENGTH)
    {
      register unsigned int key = hash (str, len);

      if (key <= MAX_HASH_VALUE)
        {
          register const char *s = wordlist[key];

          if (*str == *s && !strcmp (str + 1, s + 1))
            return s;
        }
    }
  return 0;
}

[ghost]

Thanks for your PR, @gfoidl. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

[JamesNK]

Looks great. Minor changes.

You link to this PR for more info, but I think it would be worth adding some more comments to the method about what is happening.

CI has test failures that look related to this change.

[danmoseley]

Cc @stephentoub as we were discussing perfect hashes.

[danmoseley]

I don't know if it's hot enough to be worth the same change, but it could also go here https://github.com/dotnet/runtime/blob/1d025196ae0d0f1e0b17e2e6033234e1937d997b/src/libraries/System.Net.Http/src/System/Net/Http/HttpMethod.cs#L167

[stephentoub]

I think it'd be really valuable if all of these optimized switches could just use switch, and then the C# compiler could choose an optimal implementation strategy.
dotnet/roslyn#56374
Maybe in the case of such perfect hash functions, the data size required to implement them would be prohibitive if applied to all switches over strings/spans, but it'd be worthwhile to investigate (obviously the C# compiler would need to have its own perfect hash function generation support).

These one-off manual-switch-implementations are hard to maintain.

[Tratcher]

Thoughts on casing?
#26803

[gfoidl]

For casing...current code compares with Ordinal, e.g.

aspnetcore/src/Servers/Kestrel/Core/src/Internal/Infrastructure/HttpUtilities.cs

Line 250 in ff2148b

if (firstChar == 'G' && string.Equals(value, HttpMethods.Get, StringComparison.Ordinal))

(so that should be fixed according the issue?)

Re-run the benchmark with leaving the HttpUtilities.GetKnownMethod from main as is (case-sensitive), and making the method from PR case-insensitive results in worse perf.

results

|  Method |  Method |     Mean | Ratio |
|-------- |-------- |---------:|------:|
| Default | CONNECT | 4.725 ns |  1.00 |
|      PR | CONNECT | 4.157 ns |  0.88 |
|         |         |          |       |
| Default |  DELETE | 4.171 ns |  1.00 |
|      PR |  DELETE | 4.116 ns |  0.99 |
|         |         |          |       |
| Default |     GET | 3.815 ns |  1.00 |
|      PR |     GET | 4.219 ns |  1.10 |
|         |         |          |       |
| Default |    HEAD | 3.982 ns |  1.00 |
|      PR |    HEAD | 4.119 ns |  1.04 |
|         |         |          |       |
| Default | OPTIONS | 4.529 ns |  1.00 |
|      PR | OPTIONS | 4.110 ns |  0.91 |
|         |         |          |       |
| Default |   PATCH | 4.459 ns |  1.00 |
|      PR |   PATCH | 4.364 ns |  0.98 |
|         |         |          |       |
| Default |    POST | 4.520 ns |  1.00 |
|      PR |    POST | 4.091 ns |  0.90 |
|         |         |          |       |
| Default |     PRI | 6.885 ns |  1.00 |
|      PR |     PRI | 7.188 ns |  1.04 |
|         |         |          |       |
| Default |     PUT | 3.844 ns |  1.00 |
|      PR |     PUT | 4.030 ns |  1.05 |
|         |         |          |       |
| Default |   TRACE | 4.184 ns |  1.00 |
|      PR |   TRACE | 4.030 ns |  0.96 |

Although when the current method is change to be case-insensitive too it will change the picture, and for that benchmark I just did a naive change in the code:

@@ -257,7 +257,7 @@ internal static partial class HttpUtilities
             var index = PerfectHash(value);

             if (index < (uint)WordListForPerfectHashOfMethods.Length
-                && WordListForPerfectHashOfMethods[index] == value
+                && value.Equals(s_wordListForPerfectHashOfMethods[key], StringComparison.OrdinalIgnoreCase)
                 && index < (uint)methodsLookup.Length)
             {
                 return (HttpMethod)methodsLookup[(int)index];
@@ -300,9 +300,10 @@ internal static partial class HttpUtilities
                 13, 13, 13, 13, 13, 13
             };

-            var c = MemoryMarshal.GetReference(str);
+            // Make upper-case to have a "case-insensitive" lookup.
+            var c = MemoryMarshal.GetReference(str) & ~0x20;

-            Debug.Assert(char.IsAscii(c), "Must already be valiated");
+            Debug.Assert(char.IsAscii((char)c), "Must already be valiated");

             return (uint)str.Length + associatedValues[c];
         }

[JamesNK]

If we want a case-insensitive check (is anyone in the real world actually sending requests like this?), the perf gains could be kept by adding a fallback if the original algorithm resolves to Custom. We do a second check at that point to see if it's a known value but a different case.

It's more complicated and would make Custom slightly slower, but the vast majority of HTTP requests resolve to a known value.

[JamesNK]

I think it'd be really valuable if all of these optimized switches could just use switch, and then the C# compiler could choose an optimal implementation strategy.
dotnet/roslyn#56374

+1. For this PR, if we go ahead with this custom implementation, we should link to the Roslyn issue and note that it could be removed in the future depending upon Roslyn improvements.

[gfoidl]

If we want a case-insensitive check (is anyone in the real world actually sending requests like this?)

Shall I add this?
The approach outlined looks good.

[sebastienros]

I confirm suggested implementation is faster.

[JamesNK]

If we want a case-insensitive check (is anyone in the real world actually sending requests like this?)

Shall I add this? The approach outlined looks good.

Offline discussion: Making it case insensitive can be decided and done in the future.

Thanks for the improvement!

[iSazonov]

Question: gperf is GNU General Public License. Here only the output of that tool is used. To my understanding using this here doesn't violate GPL, but please confirm.

From https://www.gnu.org/software/gperf/manual/gperf.html

3.5 The Copyright of the Output
gperf is under GPL, but that does not cause the output produced by gperf to be under GPL. The reason is that the output contains only small pieces of text that come directly from gperf's source code – only about 7 lines long, too small for being significant –, and therefore the output is not a “work based on gperf” (in the sense of the GPL version 3).

On the other hand, the output produced by gperf contains essentially all of the input file. Therefore the output is a “derivative work” of the input (in the sense of U.S. copyright law); and its copyright status depends on the copyright of the input. For most software licenses, the result is that the the output is under the same license, with the same copyright holder, as the input that was passed to gperf.

[ghost]

Hi @iSazonov. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

[IDisposable]

s/valiated/validated