Add additional groovy classloaders to ignore list.

[breedx-splk]

Groovy apps that parse a lot of scripts can generate a lot of classloaders that will ultimately end up causing the agent to cache a LOT of memory. For example, some java code that uses the Gremlin groovy script engine to dynamically create and execute scripts can reproduce this:

    GremlinGroovyScriptEngine engine = new GremlinGroovyScriptEngine();
    Bindings bindings = new SimpleBindings();
    TinkerGraph graph = TinkerGraph.open();
    GraphTraversalSource g = graph.traversal();
    bindings.put("g", g);
    for (int i = 0; i < 100000; i++) {
      engine.eval("g.V(" + i + ")", bindings);
      if(i % 250 == 0) System.out.println("Iteration " + i);
    }

I have manually confirmed that ignoring the groovy classloaders (in this PR) prevent the agent from exploding the cache and holding onto memory. I could use another brain in deciding if there could be other unintended consequences.

[mateuszrzeszutek]

Do you know when/in what cases the GroovyClassLoader is used? Looks like all our groovy tests are passing, which probably means it's not being used there. Does excluding it here mean that all groovy apps (like e.g. grails apps) will no longer be instrumented?

[laurit]

I think it is used when the groovy script is compiled on the fly.

[breedx-splk]

Do you know when/in what cases the GroovyClassLoader is used?

I'm not sure of all the cases, but in the sample code above it is used. @laurit is probably right here.

[laurit]

@breedx-splk i tried running the snipped you provided with current agent and -Xmx256m and it ran successfully to completion without an OutOfMemoryError. Is this expected?

[breedx-splk]

@breedx-splk i tried running the snipped you provided with current agent and -Xmx256m and it ran successfully to completion without an OutOfMemoryError. Is this expected?

That's surprising to me. I'm not sure if it will explode 256m, but I would have kinda expected it to. From memory -- The thing to do is to insert a sleep after the loop and while it's paused take a heap dump and look at the leaky classloaders.

[trask]

The thing to do is to insert a sleep after the loop and while it's paused take a heap dump and look at the leaky classloaders.

how are we holding onto these class loaders? generally we only hold weak references to class loaders

[breedx-splk]

how are we holding onto these class loaders? generally we only hold weak references to class loaders

I ran it again and confirmed that they are indeed weak references in the cache. So the original "holding onto memory" I guess is overstating it.

In this (contrived) example nearly all the memory is held by the classloaders in the cache, but in a more normal application I guess application memory usage will cause the cache to shrink and this isn't actually a problem?

[laurit]

In this (contrived) example nearly all the memory is held by the classloaders in the cache, but in a more normal application I guess application memory usage will cause the cache to shrink and this isn't actually a problem?

Eclipse memory analyzer removes unreachable objects from the dump. If we have a weak reference to a class loader that is otherwise unreachable it will show up in the dump. I guess this could leave an impression that the agent is increasing the heap usage, although the same objects are around even without the agent, they are just hidden form your view. In https://github.com/open-telemetry/opentelemetry-java-instrumentation/releases/tag/v1.21.0 we had a change to clear weak maps from a background thread, this could help with some agent memory usage issues. If this issue has roots in a real world application exhibiting high memory usage because of the agent then I'd suggest trying with the latest agent.
That being said excluding class loaders that can't contain stuff that should be instrumented is not a bad idea.

[trask]

That being said excluding class loaders that can't contain stuff that should be instrumented is not a bad idea.

👍