Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ISPN-12307 Improve media type parsing #8697

Merged
merged 1 commit into from Oct 5, 2020
Merged

ISPN-12307 Improve media type parsing #8697

merged 1 commit into from Oct 5, 2020

Conversation

danberindei
Copy link
Member

@danberindei danberindei commented Sep 17, 2020 •
edited

https://issues.redhat.com/browse/ISPN-12307

  • Use regular expressions for parsing to reduce allocations
    and to be closer to the spec
  • Do not remove quotes or backlashes from quoted parameter values
  • Add parsing benchmark
master Benchmark Mode Cnt Score Error Units
MediaTypeParsingBenchmark.State.parseList avgt 30 37.809 ± 0.225 ns/op
MediaTypeParsingBenchmark.State.parseNoParameter avgt 30 2.787 ± 0.019 ns/op
MediaTypeParsingBenchmark.State.parseOneParameter avgt 30 7.632 ± 0.105 ns/op
MediaTypeParsingBenchmark.State.parseOneQuotedParameter avgt 30 7.720 ± 0.154 ns/op
MediaTypeParsingBenchmark.State.parseTwoParameters avgt 30 12.389 ± 0.156 ns/op
ISPN-12307 Benchmark Mode Cnt Score Error Units
MediaTypeParsingBenchmark.State.parseList avgt 30 30.559 ± 0.220 ns/op
MediaTypeParsingBenchmark.State.parseNoParameter avgt 30 1.979 ± 0.078 ns/op
MediaTypeParsingBenchmark.State.parseOneParameter avgt 30 4.650 ± 0.027 ns/op
MediaTypeParsingBenchmark.State.parseOneQuotedParameter avgt 30 7.303 ± 0.037 ns/op
MediaTypeParsingBenchmark.State.parseTwoParameters avgt 30 8.852 ± 0.115 ns/op

@gustavocoding
Copy link

gustavocoding commented Sep 17, 2020 •
edited

So this PR makes it faster?

@tristantarrant
Copy link
Member

Checkstyle failures

@gustavocoding
Copy link

There a large perf difference in the tables above, but which one is master and which is this PR?

@danberindei danberindei force-pushed the ISPN-12307_media_type branch 2 times, most recently from 4eb20bd to 3dbc8b6 Compare September 28, 2020 17:37
@danberindei
ISPN-12307 Improve media type parsing
9c53615 
* Use regular expressions for parsing to reduce allocations
  and to be closer to the spec
* Do not remove quotes or backlashes from quoted parameter values
* Add parsing benchmark
@danberindei
Copy link
Member Author

@tristantarrant I fixed the checkstyle error

@gustavonalle the first/slow table is master, I added the branch name in the table headings.

@gustavocoding
Copy link

gustavocoding commented Sep 29, 2020 •
edited

Looks really nice @danberindei!
Before merging, let's take a chance to see if the REST benchmark is improved too

@gustavocoding
Copy link

run performance tests please
cs-rest

@ghost
Copy link

ghost commented Sep 29, 2020

Performance tests run successfully. Link to the results here.

Additional info:
Commit: 9c53615
Build number: #441
Comment body: run performance tests please\r\ncs-rest
skip ci

@gustavocoding
Copy link

No noticeable difference in the REST benchmark, probably Media Type parsing is not that important compared to other stuff that happens during a request.

@gustavocoding gustavocoding merged commit 3d9a6f0 into infinispan:master Oct 5, 2020
@gustavocoding
Copy link

Merged, thanks @danberindei !

@danberindei danberindei deleted the ISPN-12307_media_type branch October 7, 2020 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@danberindei @gustavocoding @tristantarrant