Skip to content

Don't cache JRuby runtime in static fields #707

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

Open
headius opened this issue Nov 20, 2024 · 1 comment
Open

Don't cache JRuby runtime in static fields #707

headius opened this issue Nov 20, 2024 · 1 comment
Assignees
Labels

Comments

@headius
Copy link
Contributor

headius commented Nov 20, 2024

There are a few places where we cache the JRuby runtime or runtime-specific objects in a static field, such as here with a WeakReference:

public static RuntimeInfo forRuntime(Ruby runtime) {
synchronized (RuntimeInfo.class) {
if (runtime1.get() == runtime) return info1;
RuntimeInfo cache = null;
if (runtimes != null) cache = runtimes.get(runtime);
assert cache != null : "Runtime given has not initialized JSON::Ext";
return cache;
}
}

The runtimes cached in weak references will not "leak" but it may take longer for the runtime to be collected than normal. In addition, the cost of traversing the weak reference may offset any gains from caching runtime objects.

These should be removed and cached in a runtime-safe way.

@headius
Copy link
Contributor Author

headius commented Dec 3, 2024

I improved this in #708 but eliminating all references is tricky. Basically this is being used as a way to key a cache object per-runtime that holds references to things like JSON classes, modules and exceptions. It shouldn't leak but having weak references means that dereferenced objects will survive at least one or two more GC cycles while they get cleaned up.

The cache being used may actually be more expensive than just looking up the values again. This should remain open for now and I'll return to it when I can (or someone else will) to try to eliminate more of these references.

@byroot byroot added the jruby label Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants