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

Make metaspace/class count adjustable by other buildpacks #88

Closed
3 tasks
dmikusa opened this issue Aug 16, 2021 · 1 comment · Fixed by #119
Closed
3 tasks

Make metaspace/class count adjustable by other buildpacks #88

dmikusa opened this issue Aug 16, 2021 · 1 comment · Fixed by #119
Labels
semver:minor A change requiring a minor version bump type:enhancement A general enhancement

Comments

@dmikusa
Copy link
Contributor

dmikusa commented Aug 16, 2021

What happened?

The memory calculator decides the metaspace settings for the application. It does so based on class count. This often works, but it will sometimes not work for particular types of apps or for particular Java agents. In these cases, additional tuning would be required by the user (basically to override and set metaspace).

  • What were you attempting to do?

It would be helpful if there is an interface such that buildpacks can adjust and contribute to the class count or value that the memory calculator will determine. This would allow for dynamic changes, like if a Java agent requires an additional 10% metaspace memory or if a particular application type requires more metaspace.

In addition, because the class counting occurs in the JVM provider buildpack & libjvm, it happens early in the buildpack cycle, so if you have a buildpack that provides a Java agent and it runs after the JVM provider buildpack which is very likely, then the agent isn't taken into the class count. Thus the agent would need a way to augment the class count.

Checklist

  • I have included log output.
  • The log output includes an error message.
  • I have included steps for reproduction.
@dmikusa dmikusa added semver:minor A change requiring a minor version bump type:enhancement A general enhancement labels Aug 16, 2021
@dmikusa
Copy link
Contributor Author

dmikusa commented Aug 16, 2021

At present, we use an internal variable BPI_JVM_CLASS_COUNT to communicate the class count which is determined by the buildpack when the buildpack runs to the memory calculator which runs in the launch container prior to the application starting up.

libjvm/jre.go

Lines 99 to 103 in 603755c

if c, err := count.Classes(layer.Path); err != nil {
return libcnb.Layer{}, fmt.Errorf("unable to count JVM classes\n%w", err)
} else {
layer.LaunchEnvironment.Default("BPI_JVM_CLASS_COUNT", c)
}

This variable is set as DEFAULT so it could be overwritten by another buildpack, however, it's only set a launch time so subsequent buildpacks would need to blindly overwrite it. They could not read the value calculated and increase it.

There is probably room for a richer integration here:

  • minimally, we could make BPI_JVM_CLASS_COUNT build + launch, that way it is visible at build. Then an author could read and update that value.
  • we might also look at adding an additional variable like BPI_JVM_AGENT_CLASS_COUNT where the memory calculator would take both into account
  • we could look at adding an additional environment variable like BPI_JVM_CLASS_HEADROOM that would bump up the metaspace size by an extra percent. Instead of basing off the class count, this would take the memory calculator's metaspace value and increase it by the given percent. If memory calculator determined it needs 75M of metaspace, you could then at a value of 20 which would increase the value by 20% to 90M.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:minor A change requiring a minor version bump type:enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant