Perf regression in the HTTP parser caused by long look-ahead

[lorban]

Jetty version(s)
12.0.x

Description
The changes in #11486 introduced a noticeable performance regression.

This profiler run was executed against branch 12.0.x and clearly shows that the GC is very busy.

While that profiler run was executed against branch experiment/jetty-12/revert-11486 with the only difference being the changes of #11486 were reverted, and it shows the GC is doing a lot less work.

[gregw]

@lorban Interesting, as there was good improvements in jmh including gc. But let's roll back for this release and evaluate which of the optimisations work and which do not. Maybe I implemented wrongly as compared with the jmh?

Strange that there is more allocation as the idea is that there should be none?

[lorban]

First clues of what's going on:

Allocation prof with long look-ahead vs Allocation prof without long look-ahead

Zoom on HttpParser.parseFields() and you'll notice that HttpParser.parsedHeader() (9.97% vs 1.56%) and HttpParser.takeString() (4.91% vs 9.68%) allocate more with long look-ahead while allocations in RequestHandler.headerComplete() stay rather constant (18.07% vs 19.88%).

[sbordet]

@lorban seem like the 2 runs run different code... can you double check?

[gregw]

@lorban Something is strange with that, because the changes should not have affected parseFields(), as they were almost all in quickStart, which is used for the request/response line rather than parsing fields. At least that was the part that was JMH tested... maybe I snuck in something else untested.... looking....

[lorban]

This seems reproducible:

Allocation prof with long look-ahead vs Allocation prof without long look-ahead

[gregw]

@lorban I can't see any significant changes in parseFields(), but in your flamegraphs, there is a lot of allocating of HostPortHttpField happening... so something has changed!

I'm guess the issue is that the quick start method might not be setting things up right for the Host header caching to work.

[gregw]

@lorban same thing in your second run. The code with long lookahead is allocating HostPortHttpFields in parseFields, whilst the original code is not??

[lorban]

@gregw I cannot explain it yet, but this is the data we have.

Clearly, latencies suffered, CPU profiling shows the GC is overworked so I think the allocation profile we have can be trusted... unless proven otherwise.

I'll work top-down and add some stat counters to try to understand what's going on.

[gregw]

@lorban found it! The new quick start paths are missing _fieldCache.prepare();, which is done by the normal requestLine parsing! So we are not caching host headers (or other for that matter) and will be creating lots of new ones. It should be a simple fix.... let me prepare a branch....

[gregw]

@lorban can you test #11517?

[lorban]

@gregw you nailed it, the cpu profiling of #11517 shows that the GC is now back to its normal activity.