internal/methods: add benchmarks

[dolmen]

Functions in package internal/methods could be faster. But let's start by measuring performance: add benchmarks.

[jub0bs]

Hello @dolmen! Thanks for filing this issue.

I must admit I haven't focused too much on the performance of those functions, since they're only invoked during middleware instantiation and not on the hot path (middleware invocation).

Which of those methods do you think could be faster?

• IsValid likely already is as fast as it can be, esp. now that x/net/http/httpguts: validation of header names, methods, and cookie names could be about twice as fast golang/go#66700 has been addressed.
• IsSafelisted too likely is as fast as it can be.
• IsForbidden can perhaps be improved. Is that what you had in mind?

I'd be happy to add some benchmarks around them, but I'd like to know more about what you have in mind first.

[dolmen]

A benchmark is definitely irrelevant if those functions are not used beyond the middleware instantiation.

My concerns were about the use of a map (via util.Set) just to check for the presence of a value in a set of 3 elements: using a slice and slices.Contains would simplify the implementation, and, I expect, be faster.

[dolmen]

I'm now more concerned about the use of util.Set for Config.allowedMethods.

[jub0bs]

My concerns were about the use of a map (via util.Set) just to check for the presence of a value in a set of 3 elements [...]

Fair enough. A util.Set might be overkill. I'll write a benchmark to compare the performance with that of a linear search or binary search through a []string.

I'm now more concerned about the use of util.Set for Config.allowedMethods.

This is more delicate. The cardinality of Config.allowedMethods is unbounded; storing the allowed methods in a slice could be detrimental to performance.

[jub0bs]

After thinking a bit more about this, I'm still not convinced a change in this area of the code is worth it. I'm going to close this issue for now, but I'm happy to hear counterarguments.