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

Reduce permgen use from Groovy scripts #7658

Closed
dakrone opened this issue Sep 9, 2014 · 1 comment · Fixed by #8062
Closed

Reduce permgen use from Groovy scripts #7658

dakrone opened this issue Sep 9, 2014 · 1 comment · Fixed by #8062
Assignees

Comments

@dakrone
Copy link
Member

dakrone commented Sep 9, 2014

Groovy scripts seem to be using more permgen than MVEL scripts do, especially when sent dynamically.

It would be nice if we reduce the amount of permgen used by these scripts, or make the permgen space recoverable in the event the script is not used.

@dakrone dakrone self-assigned this Sep 9, 2014
@fizx
Copy link

fizx commented Sep 23, 2014

This is because the GroovyClassLoader hangs on to every class ever created in its class cache. In benchmarks, I can only load ~500 scripts into a Java7 vm with default permgen settings before OOM/permgen. I can get a few thousand in with Java8.

#!/bin/bash
i=0
while \
curl -m 3 -X POST localhost:9200/_search\?size=0 -d @- <<EOT
{
  "query": {
    "function_score": {
      "query": { "match_all": {} },
      "script_score": {
        "script": "$(echo `date +%s`.$RANDOM)",
        "lang": "groovy"
      }
    }
  }
}
EOT
do 
  i=$((i+1))
  echo
done

echo failed after $i iterations

The following patch works for me for dynamic scripts, and I believe it shouldn't have a downside, because (1) ES has it's own Guava class cache (that actually releases classes), and (2) I can't find a codepath in ES that even indirectly uses the GroovyClassLoader's class cache.

# GroovyScriptEngineService.java#113
+ loader.clearCache();
return loader.parseClass(script, generateScriptName());

dakrone added a commit to dakrone/elasticsearch that referenced this issue Oct 14, 2014
Since we don't use the cache, it's okay to clear it entirely if needed,
Elasticsearch maintains its own cache for compiled scripts.

Adds loader.clearCache() into a listener, the listener is called when a
script is removed from the Guava cache.

This also lowers the amount of cached scripts to 100, since 500 is
around the limit some users have run into before hitting an out of
memory error in permgem.

Fixes elastic#7658
dakrone added a commit that referenced this issue Oct 14, 2014
Since we don't use the cache, it's okay to clear it entirely if needed,
Elasticsearch maintains its own cache for compiled scripts.

Adds loader.clearCache() into a listener, the listener is called when a
script is removed from the Guava cache.

This also lowers the amount of cached scripts to 100, since 500 is
around the limit some users have run into before hitting an out of
memory error in permgem.

Fixes #7658
dakrone added a commit that referenced this issue Oct 14, 2014
Since we don't use the cache, it's okay to clear it entirely if needed,
Elasticsearch maintains its own cache for compiled scripts.

Adds loader.clearCache() into a listener, the listener is called when a
script is removed from the Guava cache.

This also lowers the amount of cached scripts to 100, since 500 is
around the limit some users have run into before hitting an out of
memory error in permgem.

Fixes #7658
dakrone added a commit that referenced this issue Oct 14, 2014
Since we don't use the cache, it's okay to clear it entirely if needed,
Elasticsearch maintains its own cache for compiled scripts.

Adds loader.clearCache() into a listener, the listener is called when a
script is removed from the Guava cache.

This also lowers the amount of cached scripts to 100, since 500 is
around the limit some users have run into before hitting an out of
memory error in permgem.

Fixes #7658
mute pushed a commit to mute/elasticsearch that referenced this issue Jul 29, 2015
Since we don't use the cache, it's okay to clear it entirely if needed,
Elasticsearch maintains its own cache for compiled scripts.

Adds loader.clearCache() into a listener, the listener is called when a
script is removed from the Guava cache.

This also lowers the amount of cached scripts to 100, since 500 is
around the limit some users have run into before hitting an out of
memory error in permgem.

Fixes elastic#7658
mute pushed a commit to mute/elasticsearch that referenced this issue Jul 29, 2015
Since we don't use the cache, it's okay to clear it entirely if needed,
Elasticsearch maintains its own cache for compiled scripts.

Adds loader.clearCache() into a listener, the listener is called when a
script is removed from the Guava cache.

This also lowers the amount of cached scripts to 100, since 500 is
around the limit some users have run into before hitting an out of
memory error in permgem.

Fixes elastic#7658
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants