opentelemetry-java icon indicating copy to clipboard operation
opentelemetry-java copied to clipboard

Lazy init valid hex array

Open baolongnt opened this issue 1 month ago • 4 comments

Init of a 64k boolean array take a long time (>30ms P90) during Android app launch.

baolongnt avatar Nov 03 '25 23:11 baolongnt

CLA Signed

The committers listed above are authorized under a signed CLA.

  • :white_check_mark: login: baolongnt / name: Bao-Long Nguyen-Trong (16d68f85636202cda5add65e51239761d91c1bd6, 2770b7cc1b28321a59282f83ed10f973303f307a, 474b0e44694bcb98372b734a7372ab147ebb714e, 4e6d27424602d4816e1409389671f89676cb27ec, 5e93095dc7d192a24e3922f733e83f932b4028c9, 652457b65c6229b920a3276ae0c660d769cd1faa)

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 90.11%. Comparing base (36ca9b8) to head (4e6d274). :warning: Report is 73 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #7809      +/-   ##
============================================
- Coverage     90.18%   90.11%   -0.07%     
- Complexity     7226     7324      +98     
============================================
  Files           820      825       +5     
  Lines         21790    22051     +261     
  Branches       2135     2179      +44     
============================================
+ Hits          19651    19871     +220     
- Misses         1468     1502      +34     
- Partials        671      678       +7     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Nov 03 '25 23:11 codecov[bot]

@baolongnt we can't accept this unless you sign the CLA. Are you able to do so?

jkwatson avatar Nov 04 '25 23:11 jkwatson

@baolongnt we can't accept this unless you sign the CLA. Are you able to do so?

Thanks for looking at the PR. I did not think it will get any eyes till I mark as ready. I will have the CLA signed by then.

baolongnt avatar Nov 04 '25 23:11 baolongnt

@jkwatson @zeitlinger PR is ready. CLA signed. Thanks in advance for the review. @

baolongnt avatar Dec 13 '25 03:12 baolongnt

Have you thought about just initializing the true values explicitly, which is far fewer values than the full set of all characters. Maybe that would be a simpler way than doing things lazily like this?

Something like this, since the array will have false as the non-explicitly initialized values?

 private static boolean[] buildValidHexArray() {
    boolean[] validHex = new boolean[Character.MAX_VALUE];
    for (int i = 48; i < 103; i++) {
      validHex[i] = (48 <= i && i <= 57) || (97 <= i && i <= 102);
    }
    return validHex;
  }

jkwatson avatar Dec 13 '25 04:12 jkwatson

Have you thought about just initializing the true values explicitly, which is far fewer values than the full set of all characters. Maybe that would be a simpler way than doing things lazily like this?

I did not think about it but I am happy to switch to that. It's def a simpler change so more practical. In theory, the proposed implementation results in a faster start up but initializing 50ish booleans is much better than 64K ones.

baolongnt avatar Dec 13 '25 07:12 baolongnt

Have you thought about just initializing the true values explicitly, which is far fewer values than the full set of all characters. Maybe that would be a simpler way than doing things lazily like this?

I did not think about it but I am happy to switch to that. It's def a simpler change so more practical. In theory, the proposed implementation results in a faster start up but initializing 50ish booleans is much better than 64K ones.

If you could give it a try and see how the startup time is on Android, I think I'd prefer the smaller, simpler change.

jkwatson avatar Dec 13 '25 17:12 jkwatson