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

Support for shared GroovyClassLoader in GroovyScriptFactory #25177

Closed
change2 opened this issue Jun 2, 2020 · 4 comments
Closed

Support for shared GroovyClassLoader in GroovyScriptFactory #25177

change2 opened this issue Jun 2, 2020 · 4 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement
Milestone

Comments

@change2
Copy link

change2 commented Jun 2, 2020

Affects: Spring Framework 4.3.4


I use ScriptFactoryPostProcessor to create Groovy class bean and register in spring context. I have three thousand groovy file need to load.

I debug spring code and found that Spring uses GroovyScriptFactory to parse groovy class, but every time a new GroovyClassLoader is used and caues JVM metaspace space error.

	/**
	 * Build a {@link GroovyClassLoader} for the given {@code ClassLoader}.
	 * @param classLoader the ClassLoader to build a GroovyClassLoader for
	 * @since 4.3.3
	 */
	protected GroovyClassLoader buildGroovyClassLoader(ClassLoader classLoader) {
		return (this.compilerConfiguration != null ?
				new GroovyClassLoader(classLoader, this.compilerConfiguration) : new GroovyClassLoader(classLoader));
	}

Issue

why Every time new GroovyScriptFactory instance to parse groovy?

groovy in jave8 has metaspace bug. see
https://stackoverflow.com/questions/37301117/java-groovy-memory-leak-in-groovyclassloader

To solve this bug, we need to use global GroovyClassLoader instance to parse groovy.

how to solve ?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 2, 2020
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Jun 2, 2020
@jhoeller jhoeller self-assigned this Jun 3, 2020
@jhoeller jhoeller added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jun 3, 2020
@jhoeller jhoeller added this to the 5.2.7 milestone Jun 3, 2020
@jhoeller
Copy link
Contributor

jhoeller commented Jun 3, 2020

How are you defining those 3000 Groovy scripts? Are you using the XML scripting support or manual GroovyScriptFactory definitions?

FWIW, this is by far the largest-scale use of Spring's scripting support that I ever heard of...

@jhoeller
Copy link
Contributor

jhoeller commented Jun 3, 2020

Since the common solution for GroovyShell usage (as in our GroovyScriptEvaluator) is to externally set up a common GroovyClassLoader and pass it in as ClassLoader to all Groovy accessors, could you potentially apply a similar setup here? In Spring terms, this would mean setting up your ApplicationContext with context.setClassLoader(new GroovyClassLoader()) or the like, and then rely on Spring's Groovy support to pick up this common GroovyClassLoader directly.

Note that this won't be working quite yet: We would still have to patch GroovyScriptFactory to detect such a predetermined GroovyClassLoader, along the lines of what GroovyScriptEvaluator effectively does already. At this point I'm just asking whether such a setup would work for you.

@jhoeller
Copy link
Contributor

jhoeller commented Jun 3, 2020

A quick update: The context-level GroovyClassLoader arrangement works fine in our tests so far, in combination with a GroovyScriptFactory patch that detects it in setBeanClassLoader (in the same fashion as the GroovyShell constructor does it already). It looks like we'll be going with this solution since it is easy enough to backport to our older branches, not changing the behavior of classic class loader setups (which keep using an isolated GroovyClassLoader per scripted bean). All it should take is to explicitly configure your ApplicationContext with a setClassLoader(new GroovyClassLoader()) call, wherever you are creating or customizing the context (e.g. directly on a newly created GenericApplicationContext instance or in an ApplicationContextInitializer).

We intend to initially ship this patch in our upcoming 5.2.7 and 5.1.16 releases next week, then subsequently in 5.0.18 and 4.3.28 at a later point.

@jhoeller jhoeller changed the title GroovyScriptFactory every time new GroovyClassLoader Support for shared GroovyClassLoader in GroovyScriptFactory Jun 3, 2020
@jhoeller jhoeller added the for: backport-to-5.1.x Marks an issue as a candidate for backport to 5.1.x label Jun 3, 2020
@spring-projects-issues spring-projects-issues added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.1.x Marks an issue as a candidate for backport to 5.1.x labels Jun 3, 2020
@jhoeller
Copy link
Contributor

Please note that due to the adaptive GroovyClassLoader reuse only being available as of Groovy 2.5 - which we've upgraded to in the Spring Framework 5.1 line -, we're not backporting this further than 5.1.x on our end. Since Spring Framework 5.0.x and 4.3.x are reaching their end of life anyway, please upgrade to Spring Framework 5.1+ at your earliest convenience.

FelixFly pushed a commit to FelixFly/spring-framework that referenced this issue Aug 16, 2020
Exposes setClassLoader method in ConfigurableApplicationContext interface as obvious first-class configuration option.

Closes spring-projectsgh-25177
zx20110729 pushed a commit to zx20110729/spring-framework that referenced this issue Feb 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants