[Refactor] ImmutableOpenMap and ImmutableOpenIntMap to core lib

[nknize]

This PR refactors the heavily dependent ImmutableOpenMap and ImmutableOpenIntMap from the :server module to the :core library in preparation for refactoring other core foundation classes (e.g., StreamInput, StreamOutput, Version).

It also encapsulates the hppc library dependency by implementing the java Iterator API contracts in place of the hppc specific ObjectObjectCursor and IntObjectCursor iterators and cuts over usage to the java.util.iterator.

Microbenchmarks for compatibility w/ JFR are included to compare cpu and memory allocations.

Benchmarks show the following:

Baseline:
CPU

Result "org.opensearch.benchmark.core.common.collect.ImmutableOpenMapBenchmark.getHPPC":
  57660.709 ±(99.9%) 4644.923 ns/op [Average]
  (min, avg, max) = (53055.621, 57660.709, 62104.426), stdev = 3072.329
  CI (99.9%): [53015.785, 62305.632] (assumes normal distribution)

Memory

 Event Type                          Count  Size (bytes) 
=========================================================
 jdk.ObjectAllocationOutsideTLAB   1706889      30718576
 jdk.ObjectAllocationInNewTLAB        2372         49593

PR:
CPU

Result "org.opensearch.benchmark.core.common.collect.ImmutableOpenMapBenchmark.getJavaMap":
  52963.739 ±(99.9%) 2907.393 ns/op [Average]
  (min, avg, max) = (50511.905, 52963.739, 55639.928), stdev = 1923.061
  CI (99.9%): [50056.346, 55871.133] (assumes normal distribution)

Memory

 Event Type                          Count  Size (bytes) 
=========================================================
 jdk.ObjectAllocationOutsideTLAB   1849238      32352251
 jdk.ObjectAllocationInNewTLAB        3364         69636

CPU is faster using this PR since the modern JDKs (JDK14+) largely make the HPPC library unnecessary. The temporary ImmutableEntry (garbage collected after the iterator completes) uses 5% overhead. This overhead will go away when we remove HPPC altogether.

relates #5910

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: FAILURE ❌
• URL: https://build.ci.opensearch.org/job/gradle-check/13570/
• CommitID: 995bc34
  Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
  Is the failure a flaky test unrelated to your change?

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: FAILURE ❌
• URL: https://build.ci.opensearch.org/job/gradle-check/13571/
• CommitID: 9baea5f
  Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
  Is the failure a flaky test unrelated to your change?

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: FAILURE ❌
• URL: https://build.ci.opensearch.org/job/gradle-check/13649/
• CommitID: 638f2b6
  Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
  Is the failure a flaky test unrelated to your change?

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: UNSTABLE ❕
• TEST FAILURES:

      1 org.opensearch.cluster.allocation.AwarenessAllocationIT.testThreeZoneOneReplicaWithForceZoneValueAndLoadAwareness

• URL: https://build.ci.opensearch.org/job/gradle-check/13656/
• CommitID: dcecb87
  Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

[reta]

@nknize the allocation rates are very high for JVM version, here is the hppc allocation profile:

Benchmark                                                          (count)  Mode  Cnt       Score       Error   Units
ImmutableOpenMapBenchmark.getHPPC                                    10000  avgt   10   97339.286 ±  9278.620   ns/op
ImmutableOpenMapBenchmark.getHPPC:·gc.alloc.rate                     10000  avgt   10       0.247 ±     0.063  MB/sec
ImmutableOpenMapBenchmark.getHPPC:·gc.alloc.rate.norm                10000  avgt   10      26.520 ±     8.083    B/op
ImmutableOpenMapBenchmark.getHPPC:·gc.count                          10000  avgt   10         ≈ 0              counts

Compare that with Java one:

ImmutableOpenMapBenchmark.getJavaMap                                 10000  avgt   10  198039.787 ± 55929.563   ns/op
ImmutableOpenMapBenchmark.getJavaMap:·gc.alloc.rate                  10000  avgt   10    1126.440 ±   230.245  MB/sec
ImmutableOpenMapBenchmark.getJavaMap:·gc.alloc.rate.norm             10000  avgt   10  240088.009 ±     0.002    B/op
ImmutableOpenMapBenchmark.getJavaMap:·gc.churn.G1_Eden_Space         10000  avgt   10    1125.571 ±   251.164  MB/sec
ImmutableOpenMapBenchmark.getJavaMap:·gc.churn.G1_Eden_Space.norm    10000  avgt   10  239411.268 ±  9809.411    B/op
ImmutableOpenMapBenchmark.getJavaMap:·gc.churn.G1_Old_Gen            10000  avgt   10       0.002 ±     0.001  MB/sec
ImmutableOpenMapBenchmark.getJavaMap:·gc.churn.G1_Old_Gen.norm       10000  avgt   10       0.334 ±     0.214    B/op
ImmutableOpenMapBenchmark.getJavaMap:·gc.count                       10000  avgt   10     197.000              counts
ImmutableOpenMapBenchmark.getJavaMap:·gc.time                        10000  avgt   10     151.000                  ms

I understand that you have measured the memory heap consumption (roughly the same) but the pressure to JVM GC seems to be exceptionally high.

[nknize]

the pressure to JVM GC seems to be exceptionally high.

That makes sense because most of this (entrySet() creates a new ImmutableEntrySet whose iterator() creates a new ImmutableEntry for each ObjectObjectCursor entry) is going to be temporary object allocation. The good news is the benchmark is overly eager in it's testing. 10k objects isn't the norm for most usages of ImmutableOpenMap anyway (which is mostly used in DiscoveryNode and FieldMapper) so I don't expect the GC pressure to be operationally impactful.

[reta]

the pressure to JVM GC seems to be exceptionally high.

That makes sense because most of this (entrySet() creates a new ImmutableEntrySet whose iterator() creates a new ImmutableEntry for each ObjectObjectCursor entry) is going to be temporary object allocation. The good news is the benchmark is overly eager in it's testing. 10k objects isn't the norm for most usages of ImmutableOpenMap anyway (which is mostly used in DiscoveryNode and FieldMapper) so I don't expect the GC pressure to be operationally impactful.

The hppc has no releases since 2021 so I think we are on the right track, do you think we could move off to Eclipse Collections [1] instead for example? But yeah, if you think the operation impact is non issue (ImmutableOpenMap also leaks through clients), I am fine with that.

[1] https://www.eclipse.org/collections/javadoc/10.3.0/org/eclipse/collections/api/map/primitive/IntObjectMap.html

[nknize]

do you think we could move off to Eclipse Collections [1] instead for example

I think we could discuss for maybe a follow up PR? I'm curious what others think about this. The last benchmark I saw for Eclipse Collections measured against JDK 11 and much of the performance gains (IIRC) were explained because of specialized types (e.g., IntObjectMap) that eliminated the need for boxing primitives. This is pointed out in The State of Valhalla which is adding a lot of great improvements in this area along w/ others via Project Loom and Panama. This leans me to suggest we work towards leveraging those improvements and move the OpenSearch codebase away from these specialized types tied to third party libraries.

Maybe @uschindler or @rmuir have some thoughts on this from an OpenJDK perspective?

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: UNSTABLE ❕
• TEST FAILURES:

      1 org.opensearch.indices.replication.SegmentReplicationIT.testPitCreatedOnReplica

• URL: https://build.ci.opensearch.org/job/gradle-check/13874/
• CommitID: 508e246
  Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: UNSTABLE ❕
• TEST FAILURES:

      1 org.opensearch.remotestore.CreateRemoteIndexClusterDefaultDocRep.classMethod

• URL: https://build.ci.opensearch.org/job/gradle-check/14004/
• CommitID: 256f133
  Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

[Rishikesh1159]

@nknize can we target this for v2.8.0 and move this out of v2.7.0 scope?

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: UNSTABLE ❕
• TEST FAILURES:

      1 org.opensearch.cluster.allocation.AwarenessAllocationIT.testThreeZoneOneReplicaWithForceZoneValueAndLoadAwareness

• URL: https://build.ci.opensearch.org/job/gradle-check/14445/
• CommitID: 2b0f30c
  Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

[dblock]

Code looks good. The refactor has downstream impact on plugins packaged with the default distribution: security-analytics, k-nn, spring-data-opensearch, etc. Could you please check whether it's just a namespace change or whether there will be dragons? Maybe make the change in a couple of those (ideally all) before we merge this?

[MaxKsyunz]

Code looks good. The refactor has downstream impact on plugins packaged with the default distribution: security-analytics, k-nn, spring-data-opensearch, etc. Could you please check whether it's just a namespace change or whether there will be dragons? Maybe make the change in a couple of those (ideally all) before we merge this?

Happy to check the sql plugin.

How can I point gradle to use libraries from this branch?

[opensearch-trigger-bot]

This PR is stalled because it has been open for 30 days with no activity. Remove stalled label or comment or this will be closed in 7 days.

[opensearch-trigger-bot]

This PR was closed because it has been stalled for 7 days with no activity.