apache / pekko-http

The Streaming-first HTTP server/module of Apache Pekko
https://pekko.apache.org/
Apache License 2.0
153 stars 39 forks source link

Upstream former DynamicRuleDispatch.lookup optimization to parboiled2 #45

Closed jrudolph closed 1 year ago

jrudolph commented 1 year ago

Because we will likely remove that optimization in #14 by going back to upstream parboiled2.

mdedetrich commented 1 year ago

@jrudolph Do you want to work on this? I was thinking of having a look at it by end of week since it makes sense to upstream sooner rather than later (upstream changes take time to review)

pjfanning commented 1 year ago

Could we raise an issue in parboiled2 GitHub project? The 4 commits that @jrudolph highlighted look small enough and maybe, parboiled2 would agree to them.

mdedetrich commented 1 year ago

We could but I am not entirely sure its necessary, the performance improvements don't seem to be that controversial or large in scope and it might be almost as fast just to create a PR.

jrudolph commented 1 year ago

We shouldn't tackle that right now. It's a nice to have but not strictly required right now.

mdedetrich commented 1 year ago

We shouldn't tackle that right now. It's a nice to have but not strictly required right now.

If the reason behind not tackling it right now is due to prioritisation then thats fair

mdedetrich commented 1 year ago

Closing this since it has already been resolved (see https://github.com/sirthias/parboiled2/pull/419)