Author: radu Date: Mon Aug 31 14:02:44 2015 New Revision: 1700251 URL: http://svn.apache.org/r1700251 Log: SLING-4980 - Improve locking in the SightlyJavaCompilerService
* switched to a lock downgrading implementation in compileSource, since only compilation was affected Modified: sling/trunk/bundles/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/compiler/SightlyJavaCompilerService.java sling/trunk/bundles/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/engine/UnitLoader.java sling/trunk/bundles/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/engine/compiled/SourceIdentifier.java Modified: sling/trunk/bundles/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/compiler/SightlyJavaCompilerService.java URL: http://svn.apache.org/viewvc/sling/trunk/bundles/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/compiler/SightlyJavaCompilerService.java?rev=1700251&r1=1700250&r2=1700251&view=diff ============================================================================== --- sling/trunk/bundles/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/compiler/SightlyJavaCompilerService.java (original) +++ sling/trunk/bundles/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/compiler/SightlyJavaCompilerService.java Mon Aug 31 14:02:44 2015 @@ -49,6 +49,7 @@ import org.apache.sling.scripting.sightl import org.apache.sling.scripting.sightly.SightlyException; import org.apache.sling.scripting.sightly.impl.engine.SightlyEngineConfiguration; import org.apache.sling.scripting.sightly.impl.engine.UnitLoader; +import org.apache.sling.scripting.sightly.impl.engine.compiled.SourceIdentifier; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -124,19 +125,6 @@ public class SightlyJavaCompilerService throw new SightlyException("Cannot find class " + className + "."); } - private Object compileRepositoryJavaClass(ResourceResolver resolver, String className) { - String pojoPath = getPathFromJavaName(resolver, className); - Resource pojoResource = resolver.getResource(pojoPath); - if (pojoResource != null) { - try { - return compileSource(IOUtils.toString(pojoResource.adaptTo(InputStream.class), "UTF-8"), className); - } catch (IOException e) { - throw new SightlyException(String.format("Unable to compile class %s from %s.", className, pojoPath), e); - } - } - throw new SightlyException("Cannot find a a file corresponding to class " + className + " in the repository."); - } - /** * Compiles a class using the passed fully qualified class name and its source code. * @@ -145,37 +133,74 @@ public class SightlyJavaCompilerService * @return object instance of the class to compile * @throws CompilerException in case of any runtime exception */ - public Object compileSource(String sourceCode, String fqcn) { + public Object compileSource(SourceIdentifier sourceIdentifier, String sourceCode, String fqcn) { + boolean downgradedLock = false; writeLock.lock(); try { - if (sightlyEngineConfiguration.isDevMode()) { - String path = "/" + fqcn.replaceAll("\\.", "/") + ".java"; - OutputStream os = classLoaderWriter.getOutputStream(path); - IOUtils.write(sourceCode, os, "UTF-8"); - IOUtils.closeQuietly(os); - } - CompilationUnit compilationUnit = new SightlyCompilationUnit(sourceCode, fqcn); - - long start = System.currentTimeMillis(); - CompilationResult compilationResult = javaCompiler.compile(new CompilationUnit[]{compilationUnit}, options); - long end = System.currentTimeMillis(); - List<CompilerMessage> errors = compilationResult.getErrors(); - if (errors != null && errors.size() > 0) { - throw new CompilerException(CompilerException.CompilerExceptionCause.COMPILER_ERRORS, createErrorMsg(errors)); - } - if (LOG.isDebugEnabled()) { - LOG.debug("script compiled: {}", compilationResult.didCompile()); - LOG.debug("compilation took {}ms", end - start); - } - /** - * the class loader might have become dirty, so let the {@link ClassLoaderWriter} decide which class loader to return - */ - return classLoaderWriter.getClassLoader().loadClass(fqcn).newInstance(); + if (sourceIdentifier != null) { + if (sourceIdentifier.needsUpdate()) { + return internalCompileSource(sourceCode, fqcn); + } else { + /** + * downgrade write lock to a read lock; we don't need to recompile the class since it seems it has been recompiled by + * another thread + */ + readLock.lock(); + writeLock.unlock(); + downgradedLock = true; + return classLoaderWriter.getClassLoader().loadClass(fqcn).newInstance(); + } + } else { + return internalCompileSource(sourceCode, fqcn); + } } catch (Exception e) { throw new CompilerException(CompilerException.CompilerExceptionCause.COMPILER_ERRORS, e); } finally { - writeLock.unlock(); + if (downgradedLock) { + readLock.unlock(); + } else { + writeLock.unlock(); + } + } + } + + private Object internalCompileSource(String sourceCode, String fqcn) throws Exception { + if (sightlyEngineConfiguration.isDevMode()) { + String path = "/" + fqcn.replaceAll("\\.", "/") + ".java"; + OutputStream os = classLoaderWriter.getOutputStream(path); + IOUtils.write(sourceCode, os, "UTF-8"); + IOUtils.closeQuietly(os); + } + CompilationUnit compilationUnit = new SightlyCompilationUnit(sourceCode, fqcn); + + long start = System.currentTimeMillis(); + CompilationResult compilationResult = javaCompiler.compile(new CompilationUnit[]{compilationUnit}, options); + long end = System.currentTimeMillis(); + List<CompilerMessage> errors = compilationResult.getErrors(); + if (errors != null && errors.size() > 0) { + throw new CompilerException(CompilerException.CompilerExceptionCause.COMPILER_ERRORS, createErrorMsg(errors)); + } + if (LOG.isDebugEnabled()) { + LOG.debug("script compiled: {}", compilationResult.didCompile()); + LOG.debug("compilation took {}ms", end - start); + } + /** + * the class loader might have become dirty, so let the {@link ClassLoaderWriter} decide which class loader to return + */ + return classLoaderWriter.getClassLoader().loadClass(fqcn).newInstance(); + } + + private Object compileRepositoryJavaClass(ResourceResolver resolver, String className) { + String pojoPath = getPathFromJavaName(resolver, className); + Resource pojoResource = resolver.getResource(pojoPath); + if (pojoResource != null) { + try { + return compileSource(null, IOUtils.toString(pojoResource.adaptTo(InputStream.class), "UTF-8"), className); + } catch (IOException e) { + throw new SightlyException(String.format("Unable to compile class %s from %s.", className, pojoPath), e); + } } + throw new SightlyException("Cannot find a a file corresponding to class " + className + " in the repository."); } /** @@ -252,7 +277,6 @@ public class SightlyJavaCompilerService * @return instance of class */ private Object loadObject(String className) { - readLock.lock(); try { if (classLoaderWriter != null) { return classLoaderWriter.getClassLoader().loadClass(className).newInstance(); @@ -260,8 +284,6 @@ public class SightlyJavaCompilerService return Class.forName(className).newInstance(); } catch (Throwable t) { throw new CompilerException(CompilerException.CompilerExceptionCause.COMPILER_ERRORS, t); - } finally { - readLock.unlock(); } } Modified: sling/trunk/bundles/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/engine/UnitLoader.java URL: http://svn.apache.org/viewvc/sling/trunk/bundles/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/engine/UnitLoader.java?rev=1700251&r1=1700250&r2=1700251&view=diff ============================================================================== --- sling/trunk/bundles/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/engine/UnitLoader.java (original) +++ sling/trunk/bundles/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/engine/UnitLoader.java Mon Aug 31 14:02:44 2015 @@ -33,7 +33,6 @@ import org.apache.felix.scr.annotations. import org.apache.felix.scr.annotations.Service; import org.apache.sling.api.SlingHttpServletResponse; import org.apache.sling.api.resource.Resource; -import org.apache.sling.api.resource.ResourceMetadata; import org.apache.sling.api.resource.ResourceResolver; import org.apache.sling.api.scripting.SlingBindings; import org.apache.sling.commons.classloader.ClassLoaderWriter; @@ -94,14 +93,15 @@ public class UnitLoader { */ public RenderUnit createUnit(Resource scriptResource, Bindings bindings, RenderContextImpl renderContext) throws Exception { ResourceResolver adminResolver = renderContext.getScriptResourceResolver(); - SourceIdentifier sourceIdentifier = obtainIdentifier(scriptResource); + SourceIdentifier sourceIdentifier = new SourceIdentifier(sightlyEngineConfiguration, unitChangeMonitor, classLoaderWriter, + scriptResource, CLASS_NAME_PREFIX); Object obj; String encoding; - if (needsUpdate(sourceIdentifier)) { + if (sourceIdentifier.needsUpdate()) { unitChangeMonitor.touchScript(scriptResource.getPath()); encoding = unitChangeMonitor.getScriptEncoding(scriptResource.getPath()); String sourceCode = getSourceCodeForScript(adminResolver, sourceIdentifier, bindings, encoding, renderContext); - obj = sightlyJavaCompilerService.compileSource(sourceCode, sourceIdentifier.getFullyQualifiedName()); + obj = sightlyJavaCompilerService.compileSource(sourceIdentifier, sourceCode, sourceIdentifier.getFullyQualifiedName()); } else { encoding = unitChangeMonitor.getScriptEncoding(scriptResource.getPath()); obj = sightlyJavaCompilerService.getInstance(adminResolver, null, sourceIdentifier.getFullyQualifiedName(), false); @@ -121,10 +121,6 @@ public class UnitLoader { childTemplate = resourceFile(componentContext, CHILD_TEMPLATE_PATH); } - private SourceIdentifier obtainIdentifier(Resource resource) { - return new SourceIdentifier(resource, CLASS_NAME_PREFIX); - } - private String getSourceCodeForScript(ResourceResolver resolver, SourceIdentifier identifier, Bindings bindings, String encoding, RenderContextImpl renderContext) { String scriptSource = null; @@ -218,16 +214,4 @@ public class UnitLoader { } return ++line; } - - private boolean needsUpdate(SourceIdentifier sourceIdentifier) { - if (sightlyEngineConfiguration.isDevMode()) { - return true; - } - String slyPath = sourceIdentifier.getResource().getPath(); - long slyScriptChangeDate = unitChangeMonitor.getLastModifiedDateForScript(slyPath); - String javaCompilerPath = "/" + sourceIdentifier.getFullyQualifiedName().replaceAll("\\.", "/") + ".class"; - long javaFileDate = classLoaderWriter.getLastModified(javaCompilerPath); - return ((slyScriptChangeDate == 0 && javaFileDate > -1) || (slyScriptChangeDate > javaFileDate)); - } - } Modified: sling/trunk/bundles/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/engine/compiled/SourceIdentifier.java URL: http://svn.apache.org/viewvc/sling/trunk/bundles/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/engine/compiled/SourceIdentifier.java?rev=1700251&r1=1700250&r2=1700251&view=diff ============================================================================== --- sling/trunk/bundles/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/engine/compiled/SourceIdentifier.java (original) +++ sling/trunk/bundles/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/impl/engine/compiled/SourceIdentifier.java Mon Aug 31 14:02:44 2015 @@ -19,9 +19,11 @@ package org.apache.sling.scripting.sightly.impl.engine.compiled; -import org.apache.commons.lang.StringUtils; import org.apache.sling.api.resource.Resource; import org.apache.sling.api.resource.ResourceUtil; +import org.apache.sling.commons.classloader.ClassLoaderWriter; +import org.apache.sling.scripting.sightly.impl.compiler.UnitChangeMonitor; +import org.apache.sling.scripting.sightly.impl.engine.SightlyEngineConfiguration; /** * Identifies a Java source file in a JCR repository. @@ -32,12 +34,19 @@ public class SourceIdentifier { private final Resource resource; private final String packageName; private final String fullyQualifiedName; + private SightlyEngineConfiguration configuration; + private UnitChangeMonitor unitChangeMonitor; + private ClassLoaderWriter writer; - public SourceIdentifier(Resource resource, String classNamePrefix) { + public SourceIdentifier(SightlyEngineConfiguration configuration, UnitChangeMonitor unitChangeMonitor, ClassLoaderWriter writer, + Resource resource, String classNamePrefix) { this.resource = resource; this.className = buildClassName(resource, classNamePrefix); this.packageName = buildPackageName(resource); this.fullyQualifiedName = buildFullyQualifiedName(packageName, className); + this.configuration = configuration; + this.unitChangeMonitor = unitChangeMonitor; + this.writer = writer; } public String getClassName() { @@ -56,6 +65,17 @@ public class SourceIdentifier { return fullyQualifiedName; } + public boolean needsUpdate() { + if (configuration.isDevMode()) { + return true; + } + String slyPath = getResource().getPath(); + long slyScriptChangeDate = unitChangeMonitor.getLastModifiedDateForScript(slyPath); + String javaCompilerPath = "/" + getFullyQualifiedName().replaceAll("\\.", "/") + ".class"; + long javaFileDate = writer.getLastModified(javaCompilerPath); + return ((slyScriptChangeDate == 0 && javaFileDate > -1) || (slyScriptChangeDate > javaFileDate)); + } + private String buildFullyQualifiedName(String packageName, String className) { return packageName + "." + className; } @@ -75,10 +95,8 @@ public class SourceIdentifier { } private String getExtension(String scriptName) { - if (StringUtils.isEmpty(scriptName)) { - return null; - } int lastDotIndex = scriptName.lastIndexOf('.'); return scriptName.substring(lastDotIndex); } + }