pgjdbc/pgjdbc GitHub issues and pull requests (mirror)  
help / color / mirror / Atom feed
[pgjdbc/pgjdbc] PR #3866: perf: optimize PGInterval.getValue() by replacing String.format with StringBuilder
4+ messages / 2 participants
[nested] [flat]

* [pgjdbc/pgjdbc] PR #3866: perf: optimize PGInterval.getValue() by replacing String.format with StringBuilder
@ 2025-11-13 17:21 "vlsi (@vlsi)" <[email protected]>
  0 siblings, 0 replies; 4+ messages in thread

From: vlsi (@vlsi) @ 2025-11-13 17:21 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

TODO:
- [x] review the code


Replace String.format() and DecimalFormat with manual StringBuilder concatenation in PGInterval.getValue() to improve performance.

Fixes #3865

## benchmarks

Apple M1, Java 21, 5 forks, 3 warmup iterations, 5 measurement iterations

### Before

```
Benchmark                       (inputType)  Mode  Cnt     Score     Error   Units
getValue                               ZERO  avgt   25  1057,606 ±  24,991   ns/op
getValue:gc.alloc.rate.norm            ZERO  avgt   25  3208,007 ±   0,001    B/op
getValue                     RANDOM_SECONDS  avgt   25  1173,411 ±  17,457   ns/op
getValue:gc.alloc.rate.norm  RANDOM_SECONDS  avgt   25  3206,408 ± 107,854    B/op
getValue                        RANDOM_DAYS  avgt   25  1202,730 ±  38,124   ns/op
getValue:gc.alloc.rate.norm     RANDOM_DAYS  avgt   25  3206,408 ± 100,678    B/op
getValue                       RANDOM_YEARS  avgt   25  1200,332 ±  37,579   ns/op
getValue:gc.alloc.rate.norm    RANDOM_YEARS  avgt   25  3123,208 ± 160,827    B/op
```


### After

```
Benchmark                       (inputType)  Mode  Cnt      Score      Error   Units
getValue                               ZERO  avgt   25      7,654 ±    0,249   ns/op
getValue:gc.alloc.rate.norm            ZERO  avgt   25    128,000 ±    0,001    B/op
getValue                     RANDOM_SECONDS  avgt   25     17,306 ±    0,681   ns/op
getValue:gc.alloc.rate.norm  RANDOM_SECONDS  avgt   25    136,000 ±    0,001    B/op
getValue                        RANDOM_DAYS  avgt   25     44,907 ±   18,940   ns/op
getValue:gc.alloc.rate.norm     RANDOM_DAYS  avgt   25    156,800 ±    4,893    B/op
getValue                       RANDOM_YEARS  avgt   25     74,583 ±    1,102   ns/op
getValue:gc.alloc.rate.norm    RANDOM_YEARS  avgt   25    182,401 ±    2,446    B/op
```


## Discarded options

### Estimate capacity for `StringBuilder`

The idea is to reserve 11 chars for every non-zero field. The computation could be branchless, so it should be relatively fast.
However, it still have noticeable slowdown for small and medium inputs.

```java

### Estimage capacity for `StringBuilder`

```java
    int estimateCapacity = 11 * (
        isNonZero(years)
            + isNonZero(months)
            + isNonZero(days)
            + isNonZero(hours)
            + isNonZero(minutes)
            + isNonZero(wholeSeconds)
            + 1
    );

  private int isNonZero(int value) {
    // The logic is as follows:
    // For zero, the result is zero
    // For non-zero, either value or -value will have the highest bit set
    return (value | -value) >>> 31;
  }
```

```
Benchmark                       (inputType)  Mode  Cnt     Score     Error   Units
getValue                               ZERO  avgt   25    24,712 ±   0,494   ns/op
getValue:gc.alloc.rate.norm            ZERO  avgt   25    80,000 ±   0,001    B/op
getValue                     RANDOM_SECONDS  avgt   25    23,771 ±   1,428   ns/op
getValue:gc.alloc.rate.norm  RANDOM_SECONDS  avgt   25    96,000 ±   0,001    B/op
getValue                        RANDOM_DAYS  avgt   25    47,108 ±   3,814   ns/op
getValue:gc.alloc.rate.norm     RANDOM_DAYS  avgt   25   171,200 ±   7,339    B/op
getValue                       RANDOM_YEARS  avgt   25    67,121 ±   0,728   ns/op
getValue:gc.alloc.rate.norm    RANDOM_YEARS  avgt   25   222,400 ±   2,446    B/op
```

### Compute exact capacity for `StringBuilder`

I tried the following to estimate the string builder length exactly, however it results in slower performance overall (less allocations, but slower perf for all cases except ZERO)

```java
  private int stringSize() {
    int stringSize = stringSize(wholeSeconds) + 5;
    if (years != 0) {
      stringSize += stringSize(years) + 7;
    }
    if (months != 0) {
      stringSize += stringSize(months) + 6;
    }
    if (days != 0) {
      stringSize += stringSize(days) + 6;
    }
    if (hours != 0) {
      stringSize += stringSize(hours) + 7;
    }
    if (minutes != 0) {
      stringSize += stringSize(minutes) + 6;
    }
    if (microSeconds != 0) {
      stringSize += 7;
    }
    return stringSize;
  }

  private static int stringSize(int x) {
    int d = 1;  // assume negative (need minus sign)
    if (x >= 0) {
      d = 0;  // positive, no minus sign needed
      // The reason we prefer negative values is -Integer.MIN_VALUE can't fit into a positive range
      x = -x; // convert to negative for safe comparison
    }
    // Now x is negative, compare against negative thresholds
    if (x > -10) return 1 + d;
    if (x > -100) return 2 + d;
    if (x > -1000) return 3 + d;
    if (x > -10000) return 4 + d;
    if (x > -100000) return 5 + d;
    if (x > -1000000) return 6 + d;
    if (x > -10000000) return 7 + d;
    if (x > -100000000) return 8 + d;
    if (x > -1000000000) return 9 + d;
    return 10 + d;
  }
```



^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: [pgjdbc/pgjdbc] PR #3866: perf: optimize PGInterval.getValue() by replacing String.format with StringBuilder
@ 2025-11-15 20:39 ` "vlsi (@vlsi)" <[email protected]>
  2 siblings, 0 replies; 4+ messages in thread

From: vlsi (@vlsi) @ 2025-11-15 20:39 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

(on pgjdbc/src/main/java/org/postgresql/util/PGInterval.java)

@lukaseder, I went with a slightly different way of adding micros

^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: [pgjdbc/pgjdbc] PR #3866: perf: optimize PGInterval.getValue() by replacing String.format with StringBuilder
@ 2025-11-17 07:41 ` "vlsi (@vlsi)" <[email protected]>
  2 siblings, 0 replies; 4+ messages in thread

From: vlsi (@vlsi) @ 2025-11-17 07:41 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

I found a bug: `PGInterval(..., 1.9999998)` yields `wholeSeconds=1` and `microSeconds=1000000` which is effectively an extra second.

The bug does not affect old code much: `.getSeconds()` returned 2.0, and `getValue()` used `2.0` as well.

^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: [pgjdbc/pgjdbc] PR #3866: perf: optimize PGInterval.getValue() by replacing String.format with StringBuilder
@ 2025-11-18 08:50 ` "lukaseder (@lukaseder)" <[email protected]>
  2 siblings, 0 replies; 4+ messages in thread

From: lukaseder (@lukaseder) @ 2025-11-18 08:50 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

(on pgjdbc/src/main/java/org/postgresql/util/PGInterval.java)

@vlsi Sure. I didn't give this as much thought as you, I mainly wanted to contribute a proof of concept. Nice work!

^ permalink  raw  reply  [nested|flat] 4+ messages in thread


end of thread, other threads:[~2025-11-18 08:50 UTC | newest]

Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-11-13 17:21 [pgjdbc/pgjdbc] PR #3866: perf: optimize PGInterval.getValue() by replacing String.format with StringBuilder "vlsi (@vlsi)" <[email protected]>
2025-11-15 20:39 ` "vlsi (@vlsi)" <[email protected]>
2025-11-17 07:41 ` "vlsi (@vlsi)" <[email protected]>
2025-11-18 08:50 ` "lukaseder (@lukaseder)" <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox