vert-x3 / vert-x3/vertx-web

Suboptimal efficiency of request parameters parsing

Open
#2,425 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Java
Stars
1.2k
Forks
559
Avg merge
3d 6h
Merged PRs (30d)
7

Description

Version

4.4.2

Context

I've been running Techempower benchmarks with the type pollution agent enabled, to identify subtle scalability issues in Quarkus and Vert.x.

Do you have a reproducer?

I reproduced this issue by running the Techempower suite, but I'm pretty sure any vert.x-web application running under load with the agent would trigger this. It might be important to not have a "too simple" application as it also needs some code to attempt using ArrayList in a different context.

Type pollution report
7:	java.util.ArrayList
Count:	30848
Types:
	java.util.List
	java.lang.Iterable
	java.util.Collection
Traces:
	io.vertx.ext.web.impl.RoutingContextImpl.getQueryParams(RoutingContextImpl.java:467)
		class: java.lang.Iterable
		count: 15371
	org.postgresql.jdbc.PgResultSet.initRowBuffer(PgResultSet.java:3398)
		class: java.util.List
		count: 13064
	org.hibernate.sql.exec.internal.JdbcSelectExecutorStandardImpl.list(JdbcSelectExecutorStandardImpl.java:93)
		class: java.util.List
		count: 2397
Patch proposal / brainstorming

The method RoutingContexImpl#getQueryParams(Charset) includes the following two lines:

 Map<String, List<String>> decodedParams = new QueryStringDecoder(request.uri()).parameters();
 for (Map.Entry<String, List<String>> entry : decodedParams.entrySet()) {

I was going to suggest a workaround for the type pollution only, but then I realized: the QueryStringDecoder is allocated and then thrown away, to invoke only that single method which allocates a Map, fills it in, and then this Map is discarded as its content gets copied into the MultiMap.

That seems like a lot of allocations when what we need is only the output MultiMap; perhaps the whole logic should be refactored so to populate the MultiMap right away while parsing?

The type-pollution issue is likely to vanish as side-effect of a cleaner rewrite; a bit annoying that QueryStringDecoder is in Netty and it might need to be rewritten.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with RoutingContextImpl#getQueryParams(Charset), especially the QueryStringDecoder allocation and subsequent MultiMap copy described in the issue. Reproduce the behavior with the Techempower benchmark and type-pollution agent, then determine how request parameters can populate the MultiMap directly. Done means the parsing path avoids the unnecessary intermediate allocations while preserving parameter results.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend, performance
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.