Enable memchr feature by default?

[joshtriplett]

Should trillium-routes enable memchr by default for performance, and let people who want to avoid the dependency (and don't already have it elsewhere in their dependency tree) disable default features?

[jbr]

My concern here is less about the dependency (as trillium-http depends on memchr nonoptionally) but uncertainty about whether it represents a performance improvement. I did some benchmarking and it seems to depend on several things, including the path length and whether it can be inlined across crate boundaries, which is something only the application binary can influence with lto=fat. Using memchr represented a slight performance regression when lto=thin, but maybe that's acceptable as the default?

[joshtriplett]

Ah, I see. I would have expected the memchr crate to contain the requisite fast-paths for "this seems short" to avoid the problem of memchrnot paying off for short paths; if you have benchmarks showing cases wherememchris slower than using std, you may want to send them to thememchr` project as a performance issue to tune the heuristics. (Or, potentially, have such heuristics in trillium.)

That aside, inlining across crate boundaries should be possible with an explicit #[inline], even without lto=fat...

[jbr]

That aside, inlining across crate boundaries should be possible with an explicit #[inline], even without lto=fat...

Really? That's great news! Is that related to https://github.com/rust-lang/rust/pull/116505?

[joshtriplett]

@jbr No, that issue is about doing inlining even without #[inline]. Inlining across crates with an explicit #[inline] has been supported for a long time.

[jbr]

I did some more benchmarking and it seems like now I'm getting a slight benefit from memchr, so I'm content to make it the default (or probably just remove the feature, since trillium itself already requires memchr)