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

Investigate java-allocation-instrumenter, ensure it works as expected #2120

Closed
niloc132 opened this issue Mar 24, 2022 · 2 comments · Fixed by #2146
Closed

Investigate java-allocation-instrumenter, ensure it works as expected #2120

niloc132 opened this issue Mar 24, 2022 · 2 comments · Fixed by #2146
Assignees
Labels
Milestone

Comments

@niloc132
Copy link
Member

While the server cannot startup without this dependency, we are not setting it up on the bootclasspath as instructed, so it probably doesn't function.

We should disable the allocation.stats.enabled flag by default if it isn't actually functioning, and make the dependency compileOnly if if actually doesn't function.

If it does function, we should force an upgrade from the asm groupId to the modern org.ow2.asm groupId.

@niloc132 niloc132 added the build label Mar 24, 2022
@niloc132 niloc132 added this to the Mar 2022 milestone Mar 24, 2022
@niloc132 niloc132 self-assigned this Mar 24, 2022
@niloc132
Copy link
Member Author

niloc132 commented Mar 29, 2022

The java-allocation-instrumenter:2.0 that is currently used in DHC does not seem to work as expected (that is, Stats.getGroup("GAllocation") is empty), and is also not the same build that DHE is using (apparently packaged for DHE as allocation:allocation:2.0, I'm not clear exactly where we got this build).

The version 2.0 that can be found in maven central is missing its Premain-Class entry in META-INF/MANIFEST.MF. We could possibly try to repackage this version, or we could update to a newer version.

The latest version found in maven central is 3.3.0 (which is also the latest tag at https://github.com/google/allocation-instrumenter). After updating this dependency, I added -javaagent:/opt/deephaven/server/lib/java-allocation-instrumenter-3.3.0.jar as a JVM arg and confirmed with the groovy call println io.deephaven.base.stats.Stats.getGroup("GAllocation").items to confirm we are getting items.

My plan to close this out is to make the 3.3.0 version a compile-only dependency, disable this feature by default, and add instructions on how to enable this at runtime by adding the jar and referencing it with a -javaagent:/path/to/the/java-allocation-instrumenter.jar jvm arg.

@niloc132
Copy link
Member Author

Here's a minimal set of changes to apply to server/jetty/build.gradle to enable this javaagent at runtime, for future reference:

  1. Add a new configuration for the javaagent jar, roughly
configuration {
//...
allocation
}
  1. Add a dependency on the jar, using that new configuration (make sure it is the same version that stats is using)
dependencies {
//...
allocation 'com.google.code.java-allocation-instrumenter:java-allocation-instrumenter:3.3.0'
}
  1. Add this jar as a javaagent to the jvm args. To do this with JavaExec in its current form, we just append a new string to the extraJvmArgs array. Note that 3.3.0 is a shaded jar, and includes its dependencies, so we only need the very first element in that configuration:
extraJvmArgs += ["-javaagent:${configurations.allocation.iterator().next()}"]
  1. Enable allocation stats, by also adding the system property to the extraJvmArgs: -Dallocation.stats.enabled=true

Confirmed by running println io.deephaven.base.stats.Stats.getGroup("GAllocation").items on the jetty server after making this change.

niloc132 added a commit to niloc132/deephaven-core that referenced this issue Mar 30, 2022
Also adds guava as an explicit dependency to other modules, which
previously implicitly picked up guava from the old
java-allocation-tracker library version.

Fixes deephaven#2120
niloc132 added a commit to niloc132/deephaven-core that referenced this issue Mar 30, 2022
Also adds guava as an explicit dependency to other modules, which
previously implicitly picked up guava from the old
java-allocation-tracker library version.

Fixes deephaven#2120
niloc132 added a commit to niloc132/deephaven-core that referenced this issue Mar 30, 2022
Also adds guava as an explicit dependency to other modules, which
previously implicitly picked up guava from the old
java-allocation-tracker library version.

Fixes deephaven#2120
niloc132 added a commit that referenced this issue Mar 30, 2022
…2146)

Also adds guava as an explicit dependency to other modules, which
previously implicitly picked up guava from the old
java-allocation-tracker library version.

Fixes #2120
mofojed pushed a commit that referenced this issue Jul 17, 2024
## Release notes
https://github.com/deephaven/web-client-ui/releases/tag/v0.86.0

### Features

* Add option to disable WebGL rendering
([#2134](deephaven/web-client-ui#2134))
([011eb33](deephaven/web-client-ui@011eb33))
* Core plugins refactor, XComponent framework
([#2150](deephaven/web-client-ui#2150))
([2571fad](deephaven/web-client-ui@2571fad))
* IrisGridTheme iconSize
([#2123](deephaven/web-client-ui#2123))
([58ee88d](deephaven/web-client-ui@58ee88d)),
closes [#885](deephaven/web-client-ui#885)
* Partitioned Table UI Enhancements
([#2110](deephaven/web-client-ui#2110))
([de5ce40](deephaven/web-client-ui@de5ce40)),
closes [#2079](deephaven/web-client-ui#2079)
[#2066](deephaven/web-client-ui#2066)
[#2103](deephaven/web-client-ui#2103)
[#2104](deephaven/web-client-ui#2104)
[#2105](deephaven/web-client-ui#2105)
[#2106](deephaven/web-client-ui#2106)
[#2107](deephaven/web-client-ui#2107)
[#2108](deephaven/web-client-ui#2108)
[#2109](deephaven/web-client-ui#2109)
[#2049](deephaven/web-client-ui#2049)
[#2120](deephaven/web-client-ui#2120)
[#1904](deephaven/web-client-ui#1904)


### Bug Fixes

* error when edited cell is out of grid viewport
([#2148](deephaven/web-client-ui#2148))
([3fccd43](deephaven/web-client-ui@3fccd43)),
closes [#2087](deephaven/web-client-ui#2087)

## Release notes
https://github.com/deephaven/web-client-ui/releases/tag/v0.85.2


### Bug Fixes

* Fix missing scrim background on LoadingOverlay
([#2098](deephaven/web-client-ui#2098))
([c9ed895](deephaven/web-client-ui@c9ed895))

## Release notes
https://github.com/deephaven/web-client-ui/releases/tag/v0.85.1

##
[0.85.1](deephaven/web-client-ui@v0.85.0...v0.85.1)
(2024-07-08)


### Bug Fixes

* re-export remaining types needed by dh ui from @react-types/shared
([#2132](deephaven/web-client-ui#2132))
([2119a61](deephaven/web-client-ui@2119a61))



## Release notes
https://github.com/deephaven/web-client-ui/releases/tag/v0.85.0

### Features

* ComboBox - @deephaven/jsapi-components
([#2077](deephaven/web-client-ui#2077))
([115e057](deephaven/web-client-ui@115e057)),
closes [#2074](deephaven/web-client-ui#2074)


### Bug Fixes

* Allow ComboBox to accept the FocusableRef for ref
([#2121](deephaven/web-client-ui#2121))
([8fe9bad](deephaven/web-client-ui@8fe9bad))
* Ref was not being passed through for Picker
([#2122](deephaven/web-client-ui#2122))
([a11e2ce](deephaven/web-client-ui@a11e2ce))

Co-authored-by: deephaven-internal <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant