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.

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

MethodProbes should make use of MethodHandles. [HZ-3024] #25244

Closed
pveentjer opened this issue Aug 17, 2023 · 1 comment · Fixed by #25279
Closed

MethodProbes should make use of MethodHandles. [HZ-3024] #25244

pveentjer opened this issue Aug 17, 2023 · 1 comment · Fixed by #25279

Comments

@pveentjer
Copy link
Contributor

pveentjer commented Aug 17, 2023

Currently, reflection is used to access methods. The problem is that this causes litter for methods that return a primitive value.

With MethodHandles, this litter can be prevented.

@github-actions
Copy link
Contributor

Internal Jira issue: HZ-3024

@github-actions github-actions bot changed the title MethodProbes should make use of MethodHandles. MethodProbes should make use of MethodHandles. [HZ-3024] Aug 30, 2023
pveentjer pushed a commit that referenced this issue Oct 3, 2023
…5279)

When accessing the data provided by `MethodProbe`s, currently core Java
reflection is used.

I've replaced this with
[`MethodHandle`s](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/invoke/MethodHandle.html):

- with a `MethodHandle` it's possible to directly access a primitive,
without boxing/unboxing - reducing object creation (and therefore
garbage collection overhead)
- it allows the handle to be constructed once and used over and over
again allowing it to be more efficient

**Benchmarks:**
I benchmarked
([`MethodProbeBenchmark`](hazelcast/internal-benchmarks#40))
the improvement made to a the usage of a `MethodProbe`:

| Access Type | Method Type | ns/op | GC Alloc Rate (MB/s) | GC Alloc
Rate Norm (b/op) | GC Count | GC Time (ms) |
| ------------- | ------------- | ------------- | ------------- |
------------- | ------------- | ------------- |
| Reflection | `long` primitive | 6.8 | 3368 | 24 | 56 | 25 |
| `MethodHandle` | `long` primitive | 4.3 | 0.05 | 0 | 0 | 0 |
| Reflection | `Long` Object | 6.7 | 3419 | 24 | 62 | 23 |
| `MethodHandle` | `Long` Object | 2.1 | 0.05 | 0 | 0 | 0 |

The improvements are:
- faster execution time
- an order of magnitude more efficient in terms of garbage collection

This benchmark includes the overhead of the method operation itself - so
is reflective of the real-world observable performance improvement,
rather than a theoretical comparison of the two implementations.

More information on the implementation can be found in the Javadoc of
[`MethodProbe`](https://github.com/hazelcast/hazelcast/blob/a00165c95bea4641977ae2eaf58aadd5ac42a779/hazelcast/src/main/java/com/hazelcast/internal/metrics/impl/MethodProbe.java).

**Summary of Changes**

- Made `MethodProbe` use `MethodHandle`s
- Migrated static `int` constants in `ProbeUtils` to `ProbeType` `enum`
  - Allows properties to be added
  - Allows static methods to be moved into the object
  - Tidy `flatten`ing logic to avoid a `List.contains` iteration
- Updated `FieldProbe` to suit refactoring

Fixes [#25244](#25244)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants