Skip to content

Commit

Permalink
[MNG-6697] New fast model interpolator not using reflection (#261)
Browse files Browse the repository at this point in the history 
* [MNG-6697] New fast model interpolator not using reflection

* [MNG-6697] Fix management key in case a field has been modified

* [MNG-6697] Remove the unused FIELDS_CACHE and make the InnerInterpolator private

* [MNG-6697] Clean up the code to remove a few warnings
  • Loading branch information
@gnodet @olamy
gnodet authored and olamy committed Jul 24, 2019
1 parent 74e8532 commit 690841e
Show file tree
Hide file tree
Showing 5 changed files with 1,547 additions and 133 deletions.
4 changes: 2 additions & 2 deletions ...del-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilderFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import org.apache.maven.model.inheritance.DefaultInheritanceAssembler;
import org.apache.maven.model.inheritance.InheritanceAssembler;
import org.apache.maven.model.interpolation.ModelInterpolator;
import org.apache.maven.model.interpolation.StringSearchModelInterpolator;
import org.apache.maven.model.interpolation.StringVisitorModelInterpolator;
import org.apache.maven.model.io.DefaultModelReader;
import org.apache.maven.model.io.ModelReader;
import org.apache.maven.model.locator.DefaultModelLocator;
Expand Down Expand Up @@ -126,7 +126,7 @@ protected ModelInterpolator newModelInterpolator()
{
UrlNormalizer normalizer = newUrlNormalizer();
PathTranslator pathTranslator = newPathTranslator();
return new StringSearchModelInterpolator().setPathTranslator( pathTranslator ).setUrlNormalizer( normalizer );
return new StringVisitorModelInterpolator().setPathTranslator( pathTranslator ).setUrlNormalizer( normalizer );
}

protected ModelValidator newModelValidator()
Expand Down
81 changes: 2 additions & 79 deletions .../main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,11 @@

import org.apache.maven.model.Model;
import org.apache.maven.model.building.ModelBuildingRequest;
import org.apache.maven.model.building.ModelProblem.Severity;
import org.apache.maven.model.building.ModelProblem.Version;
import org.apache.maven.model.building.ModelProblemCollector;
import org.apache.maven.model.building.ModelProblemCollectorRequest;
import org.apache.maven.model.path.PathTranslator;
import org.apache.maven.model.path.UrlNormalizer;
import org.codehaus.plexus.interpolation.AbstractValueSource;
import org.codehaus.plexus.interpolation.InterpolationException;
import org.codehaus.plexus.interpolation.InterpolationPostProcessor;
import org.codehaus.plexus.interpolation.Interpolator;
import org.codehaus.plexus.interpolation.MapBasedValueSource;
import org.codehaus.plexus.interpolation.ObjectBasedValueSource;
import org.codehaus.plexus.interpolation.PrefixAwareRecursionInterceptor;
Expand Down Expand Up @@ -93,14 +88,8 @@ public abstract class AbstractStringBasedModelInterpolator
@Inject
private UrlNormalizer urlNormalizer;

private Interpolator interpolator;

private RecursionInterceptor recursionInterceptor;

public AbstractStringBasedModelInterpolator()
{
interpolator = createInterpolator();
recursionInterceptor = new PrefixAwareRecursionInterceptor( PROJECT_PREFIXES );
}

public AbstractStringBasedModelInterpolator setPathTranslator( PathTranslator pathTranslator )
Expand Down Expand Up @@ -218,75 +207,9 @@ protected List<? extends InterpolationPostProcessor> createPostProcessors( final
return processors;
}

protected String interpolateInternal( String src, List<? extends ValueSource> valueSources,
List<? extends InterpolationPostProcessor> postProcessors,
ModelProblemCollector problems )
{
if ( !src.contains( "${" ) )
{
return src;
}

String result = src;
synchronized ( this )
{

for ( ValueSource vs : valueSources )
{
interpolator.addValueSource( vs );
}

for ( InterpolationPostProcessor postProcessor : postProcessors )
{
interpolator.addPostProcessor( postProcessor );
}

try
{
try
{
result = interpolator.interpolate( result, recursionInterceptor );
}
catch ( InterpolationException e )
{
problems.add( new ModelProblemCollectorRequest( Severity.ERROR, Version.BASE )
.setMessage( e.getMessage() ).setException( e ) );
}

interpolator.clearFeedback();
}
finally
{
for ( ValueSource vs : valueSources )
{
interpolator.removeValuesSource( vs );
}

for ( InterpolationPostProcessor postProcessor : postProcessors )
{
interpolator.removePostProcessor( postProcessor );
}
}
}

return result;
}

protected RecursionInterceptor getRecursionInterceptor()
{
return recursionInterceptor;
}

protected void setRecursionInterceptor( RecursionInterceptor recursionInterceptor )
{
this.recursionInterceptor = recursionInterceptor;
}

protected abstract Interpolator createInterpolator();

protected final Interpolator getInterpolator()
protected RecursionInterceptor createRecursionInterceptor()
{
return interpolator;
return new PrefixAwareRecursionInterceptor( PROJECT_PREFIXES );
}

}
130 changes: 78 additions & 52 deletions ...der/src/main/java/org/apache/maven/model/interpolation/StringSearchModelInterpolator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@
import org.apache.maven.model.building.ModelProblem.Version;
import org.apache.maven.model.building.ModelProblemCollector;
import org.apache.maven.model.building.ModelProblemCollectorRequest;
import org.codehaus.plexus.interpolation.InterpolationException;
import org.codehaus.plexus.interpolation.InterpolationPostProcessor;
import org.codehaus.plexus.interpolation.Interpolator;
import org.codehaus.plexus.interpolation.RecursionInterceptor;
import org.codehaus.plexus.interpolation.StringSearchInterpolator;
import org.codehaus.plexus.interpolation.ValueSource;

Expand All @@ -39,6 +40,7 @@
import java.security.PrivilegedAction;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
Expand All @@ -60,6 +62,10 @@ public class StringSearchModelInterpolator
new ConcurrentHashMap<>( 80, 0.75f, 2 );
// Empirical data from 3.x, actual =40

private interface InnerInterpolator
{
String interpolate( String value );
}

@Override
public Model interpolateModel( Model model, File projectDir, ModelBuildingRequest config,
Expand All @@ -71,32 +77,62 @@ public Model interpolateModel( Model model, File projectDir, ModelBuildingReques
}

protected void interpolateObject( Object obj, Model model, File projectDir, ModelBuildingRequest config,
ModelProblemCollector problems )
final ModelProblemCollector problems )
{
try
{
List<? extends ValueSource> valueSources = createValueSources( model, projectDir, config, problems );
List<? extends InterpolationPostProcessor> postProcessors =
createPostProcessors( model, projectDir, config );
List<? extends ValueSource> valueSources = createValueSources( model, projectDir, config, problems );
List<? extends InterpolationPostProcessor> postProcessors =
createPostProcessors( model, projectDir, config );

InterpolateObjectAction action =
new InterpolateObjectAction( obj, valueSources, postProcessors, this, problems );
InnerInterpolator innerInterpolator = createInterpolator( valueSources, postProcessors, problems );

PrivilegedAction<Object> action;
action = new InterpolateObjectAction( obj, valueSources, postProcessors, innerInterpolator, problems );
AccessController.doPrivileged( action );

AccessController.doPrivileged( action );
}
finally
{
getInterpolator().clearAnswers();
}
}

@Override
protected Interpolator createInterpolator()
private InnerInterpolator createInterpolator( List<? extends ValueSource> valueSources,
List<? extends InterpolationPostProcessor> postProcessors,
final ModelProblemCollector problems )
{
StringSearchInterpolator interpolator = new StringSearchInterpolator();
final Map<String, String> cache = new HashMap<>();
final StringSearchInterpolator interpolator = new StringSearchInterpolator();
interpolator.setCacheAnswers( true );

return interpolator;
for ( ValueSource vs : valueSources )
{
interpolator.addValueSource( vs );
}
for ( InterpolationPostProcessor postProcessor : postProcessors )
{
interpolator.addPostProcessor( postProcessor );
}
final RecursionInterceptor recursionInterceptor = createRecursionInterceptor();
return new InnerInterpolator()
{
@Override
public String interpolate( String value )
{
if ( value != null && value.contains( "${" ) )
{
String c = cache.get( value );
if ( c == null )
{
try
{
c = interpolator.interpolate( value, recursionInterceptor );
}
catch ( InterpolationException e )
{
problems.add( new ModelProblemCollectorRequest( Severity.ERROR, Version.BASE )
.setMessage( e.getMessage() ).setException( e ) );
}
cache.put( value, c );
}
return c;
}
return value;
}
};
}

private static final class InterpolateObjectAction
Expand All @@ -105,7 +141,7 @@ private static final class InterpolateObjectAction

private final LinkedList<Object> interpolationTargets;

private final StringSearchModelInterpolator modelInterpolator;
private final InnerInterpolator interpolator;

private final List<? extends ValueSource> valueSources;

Expand All @@ -115,15 +151,15 @@ private static final class InterpolateObjectAction

InterpolateObjectAction( Object target, List<? extends ValueSource> valueSources,
List<? extends InterpolationPostProcessor> postProcessors,
StringSearchModelInterpolator modelInterpolator, ModelProblemCollector problems )
InnerInterpolator interpolator, ModelProblemCollector problems )
{
this.valueSources = valueSources;
this.postProcessors = postProcessors;

this.interpolationTargets = new LinkedList<>();
interpolationTargets.add( target );

this.modelInterpolator = modelInterpolator;
this.interpolator = interpolator;

this.problems = problems;
}
Expand All @@ -144,7 +180,7 @@ public Object run()

private String interpolate( String value )
{
return modelInterpolator.interpolateInternal( value, valueSources, postProcessors, problems );
return interpolator.interpolate( value );
}

private void traverseObjectWithParents( Class<?> cls, Object target )
Expand Down Expand Up @@ -297,40 +333,30 @@ abstract static class CacheField
CacheField( Field field )
{
this.field = field;
field.setAccessible( true );
}

void interpolate( Object target, InterpolateObjectAction interpolateObjectAction )
{
synchronized ( field )
try
{
boolean isAccessible = field.isAccessible();
field.setAccessible( true );
try
{
doInterpolate( target, interpolateObjectAction );
}
catch ( IllegalArgumentException e )
{
interpolateObjectAction.problems.add(
new ModelProblemCollectorRequest( Severity.ERROR, Version.BASE ).setMessage(
"Failed to interpolate field3: " + field + " on class: "
+ field.getType().getName() ).setException(
e ) ); // TODO Not entirely the same message
}
catch ( IllegalAccessException e )
{
interpolateObjectAction.problems.add(
new ModelProblemCollectorRequest( Severity.ERROR, Version.BASE ).setMessage(
"Failed to interpolate field4: " + field + " on class: "
+ field.getType().getName() ).setException( e ) );
}
finally
{
field.setAccessible( isAccessible );
}
doInterpolate( target, interpolateObjectAction );
}
catch ( IllegalArgumentException e )
{
interpolateObjectAction.problems.add(
new ModelProblemCollectorRequest( Severity.ERROR, Version.BASE ).setMessage(
"Failed to interpolate field3: " + field + " on class: "
+ field.getType().getName() ).setException(
e ) ); // TODO Not entirely the same message
}
catch ( IllegalAccessException e )
{
interpolateObjectAction.problems.add(
new ModelProblemCollectorRequest( Severity.ERROR, Version.BASE ).setMessage(
"Failed to interpolate field4: " + field + " on class: "
+ field.getType().getName() ).setException( e ) );
}


}

abstract void doInterpolate( Object target, InterpolateObjectAction ctx )
Expand Down
Loading

0 comments on commit 690841e

Please sign in to comment.