Inline peek_ahead to fix 186 and remove iter::peek_ahead as it is now unused.

seanmonstar / httparse

A push parser for the HTTP 1.x protocol in Rust.

https://docs.rs/httparse

Apache License 2.0

590 stars 115 forks source link

Inline peek_ahead to fix 186 and remove iter::peek_ahead as it is now unused. #188

Closed hkBst closed 3 days ago

hkBst commented 1 month ago

Fixes #186

hkBst commented 1 month ago

Actually, is there really any reason for this complicated dance? Why not just match on "POST "? It seems that would probably do fewer bounds checks as well. Or if it does one for GET and then another one for POST, maybe it would work to match on bytes.len()?

seanmonstar commented 1 month ago

I can say that method was very carefully adjusted to improve performance significantly.

hkBst commented 3 weeks ago

Right, well this patch just inlines what the compiler was asked to inline, possibly a slightly more efficient version, so there should be no performance regression.

Is there anything you dislike about this approach?

seanmonstar commented 3 weeks ago

Hm, could we just make peek_ahead unsafe and revert back to using cursor.add(n) once, adding a comment that the caller must be sure n is within the range? I think that'd keep the one place it's called a little cleaner.

hkBst commented 3 days ago

Alright, after some thought I agree with you that this is not very nice, since we are breaking out of the Bytes abstraction, so I am closing this in favor of pull #191.