Hand rolled parser for SlsVersionType.RELEASE (aka x.y.z) #463

iamdanfox · 2021-06-15T01:31:29Z

Before this PR: 416 bytes / op

From 1 minute JFR, the hottest method in my application is currently java.util.regex, and the biggest proportion of these Matcher constructions are coming from the lovely OrderableSlsVersion#safeValueOf

This seems a bit silly, given that most of the time we're trying to parse three numbers with dots in between them.

Benchmark                                                         (versionString)  Mode  Cnt     Score      Error   Units
SlsVersionBenchmark.safeValueOf                                           RELEASE  avgt    3   176.778 ±   30.231   ns/op
SlsVersionBenchmark.safeValueOf:·gc.alloc.rate                            RELEASE  avgt    3  7690.289 ± 1225.982  MB/sec
SlsVersionBenchmark.safeValueOf:·gc.alloc.rate.norm                       RELEASE  avgt    3   416.021 ±    0.007    B/op
SlsVersionBenchmark.safeValueOf:·gc.churn.G1_Eden_Space                   RELEASE  avgt    3  7751.897 ± 1363.284  MB/sec
SlsVersionBenchmark.safeValueOf:·gc.churn.G1_Eden_Space.norm              RELEASE  avgt    3   419.352 ±   10.138    B/op
SlsVersionBenchmark.safeValueOf:·gc.churn.G1_Survivor_Space               RELEASE  avgt    3     0.035 ±    0.064  MB/sec
SlsVersionBenchmark.safeValueOf:·gc.churn.G1_Survivor_Space.norm          RELEASE  avgt    3     0.002 ±    0.004    B/op
SlsVersionBenchmark.safeValueOf:·gc.count                                 RELEASE  avgt    3   184.000             counts
SlsVersionBenchmark.safeValueOf:·gc.time                                  RELEASE  avgt    3   103.000                 ms

After this PR: 104 bytes / op

==COMMIT_MSG==
Deserializing a verison of the form x.y.z now allocates less. This should mean GC algorithms have less work to do.
==COMMIT_MSG==


Benchmark                                                         (versionString)  Mode  Cnt     Score       Error   Units
SlsVersionBenchmark.safeValueOf                                           RELEASE  avgt    3    55.592 ±   109.978   ns/op
SlsVersionBenchmark.safeValueOf:·gc.alloc.rate                            RELEASE  avgt    3  6158.567 ± 11630.568  MB/sec
SlsVersionBenchmark.safeValueOf:·gc.alloc.rate.norm                       RELEASE  avgt    3   104.006 ±     0.002    B/op
SlsVersionBenchmark.safeValueOf:·gc.churn.G1_Eden_Space                   RELEASE  avgt    3  6200.873 ± 12743.698  MB/sec
SlsVersionBenchmark.safeValueOf:·gc.churn.G1_Eden_Space.norm              RELEASE  avgt    3   104.656 ±    22.149    B/op
SlsVersionBenchmark.safeValueOf:·gc.churn.G1_Survivor_Space               RELEASE  avgt    3     0.019 ±     0.067  MB/sec
SlsVersionBenchmark.safeValueOf:·gc.churn.G1_Survivor_Space.norm          RELEASE  avgt    3    ≈ 10⁻³                B/op
SlsVersionBenchmark.safeValueOf:·gc.count                                 RELEASE  avgt    3   147.000              counts
SlsVersionBenchmark.safeValueOf:·gc.time                                  RELEASE  avgt    3    88.000                  ms

Possible downsides?

changelog-app · 2021-06-15T16:15:25Z

Generate changelog in `changelog/@unreleased`

Type

Description

Parsing the simplest x.y.z type of OrderableSlsVersion no longer uses regexes. Allocation is reduced by about 75%

Check the box to generate changelog(s)

Generate changelog entry

iamdanfox · 2021-06-15T16:19:30Z

sls-versions/src/main/java/com/palantir/sls/versions/Parsers.java

+ * We bit-pack these two integer values into a single long using {@link Parsers#ok} and {@link Parsers#fail} functions
+ * because primitive longs live on the stack and don't impact GC.
+ */
+final class Parsers {


all these functions just got moved from SlsVersionMatcherParser.java - no changes made here.

sls-versions/src/main/java/com/palantir/sls/versions/Parsers.java

CRogers · 2021-06-21T10:40:13Z

sls-versions/src/main/java/com/palantir/sls/versions/ReleaseVersionParser.java

+        state = Parsers.literalDot(string, Parsers.getIndex(state));
+        if (Parsers.failed(state)) {
+            return null;
+        }


This is just begging for some kind of monadic bind abstraction... but obviously that would require a lot of allocation in Java.

totally agree

CRogers · 2021-06-21T10:47:16Z

sls-versions/src/main/java/com/palantir/sls/versions/ReleaseVersionParser.java

+        if (Parsers.failed(state)) {
+            return null;
+        }
+        int third = Parsers.getResult(state);


I was thinking maybe you could make this less duplicated by having a for loop - but then you'd need to either make an an array or have Int3MatchResult be mutable/have a builder.

You could also group literalDot/number into a method, but it would need to return Integer so it can be nullable, which would I guess would be bad.

CRogers · 2021-06-21T10:51:11Z

👍

svc-autorelease · 2021-06-21T15:09:42Z

Released 0.18.0

policy-bot bot requested a review from tetigi June 15, 2021 01:31

iamdanfox force-pushed the dfox/hand-roll branch from dfa2b1a to ef64650 Compare June 15, 2021 01:35

iamdanfox removed the request for review from tetigi June 15, 2021 01:37

iamdanfox changed the base branch from dfox/less-regex to develop June 15, 2021 16:04

iamdanfox added no changelog and removed no changelog labels Jun 15, 2021

iamdanfox commented Jun 15, 2021

View reviewed changes

iamdanfox changed the title ~~[mainly just for fun] Hand rolled parser for SlsVersionType.RELEASE (aka x.y.z)~~ Hand rolled parser for SlsVersionType.RELEASE (aka x.y.z) Jun 16, 2021

hand roll

9d24113

iamdanfox force-pushed the dfox/hand-roll branch from 10c936f to 9d24113 Compare June 16, 2021 21:28

iamdanfox and others added 2 commits June 16, 2021 22:35

javadoc

aebe397

Add generated changelog entries

278bb8b

CRogers reviewed Jun 21, 2021

View reviewed changes

sls-versions/src/main/java/com/palantir/sls/versions/Parsers.java Outdated Show resolved Hide resolved

constant

c2de3aa

CRogers reviewed Jun 21, 2021

View reviewed changes

iamdanfox added autorelease merge when ready labels Jun 21, 2021

bulldozer-bot bot merged commit 4e7b82d into develop Jun 21, 2021

schlosna deleted the dfox/hand-roll branch July 14, 2022 19:54

schlosna mentioned this pull request Jul 14, 2022

Optimize UserAgent version validation palantir/conjure-java-runtime-api#864

Merged

aldexis mentioned this pull request Aug 25, 2025

Streamline and improve performance for orderable sls version parsing #1185

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Hand rolled parser for SlsVersionType.RELEASE (aka x.y.z) #463

Hand rolled parser for SlsVersionType.RELEASE (aka x.y.z) #463

Uh oh!

iamdanfox commented Jun 15, 2021 •

edited

Loading

Uh oh!

changelog-app bot commented Jun 15, 2021 •

edited by iamdanfox

Loading

Uh oh!

iamdanfox Jun 15, 2021

Uh oh!

Uh oh!

CRogers Jun 21, 2021

Uh oh!

iamdanfox Jun 21, 2021

Uh oh!

CRogers Jun 21, 2021

Uh oh!

CRogers commented Jun 21, 2021

Uh oh!

svc-autorelease commented Jun 21, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Hand rolled parser for SlsVersionType.RELEASE (aka x.y.z) #463

Hand rolled parser for SlsVersionType.RELEASE (aka x.y.z) #463

Uh oh!

Conversation

iamdanfox commented Jun 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before this PR: 416 bytes / op

After this PR: 104 bytes / op

Possible downsides?

Uh oh!

changelog-app bot commented Jun 15, 2021 • edited by iamdanfox Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Generate changelog in changelog/@unreleased

Uh oh!

iamdanfox Jun 15, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

CRogers Jun 21, 2021

Choose a reason for hiding this comment

Uh oh!

iamdanfox Jun 21, 2021

Choose a reason for hiding this comment

Uh oh!

CRogers Jun 21, 2021

Choose a reason for hiding this comment

Uh oh!

CRogers commented Jun 21, 2021

Uh oh!

svc-autorelease commented Jun 21, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

iamdanfox commented Jun 15, 2021 •

edited

Loading

changelog-app bot commented Jun 15, 2021 •

edited by iamdanfox

Loading

Generate changelog in `changelog/@unreleased`