You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by si...@apache.org on 2008/09/28 06:52:54 UTC

svn commit: r699773 - in /maven/components/trunk/maven-project/src/main/java/org/apache/maven/project: DefaultMavenProjectBuilder.java artifact/MavenMetadataSource.java

Author: sisbell
Date: Sat Sep 27 21:52:53 2008
New Revision: 699773

URL: http://svn.apache.org/viewvc?rev=699773&view=rev
Log:
Removed use of workspace from project builder. In the build of trunk, there were about 50K of calls from MavenMetadataSource to the project builder. I put a simple hashmap cache in the metadata source to reduce calls to dozens.

Modified:
    maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/DefaultMavenProjectBuilder.java
    maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java

Modified: maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/DefaultMavenProjectBuilder.java
URL: http://svn.apache.org/viewvc/maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/DefaultMavenProjectBuilder.java?rev=699773&r1=699772&r2=699773&view=diff
==============================================================================
--- maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/DefaultMavenProjectBuilder.java (original)
+++ maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/DefaultMavenProjectBuilder.java Sat Sep 27 21:52:53 2008
@@ -132,11 +132,7 @@
     public MavenProject build( File projectDescriptor, ProjectBuilderConfiguration config )
         throws ProjectBuildingException
     {
-        MavenProject project = projectWorkspace.getProject( projectDescriptor );
-
-        if ( project == null )
-        {
-            project = readModelFromLocalPath( "unknown", projectDescriptor, new PomArtifactResolver(
+            MavenProject project = readModelFromLocalPath( "unknown", projectDescriptor, new PomArtifactResolver(
                 config.getLocalRepository(), repositoryHelper.buildArtifactRepositories(
                 getSuperProject( config, projectDescriptor, true ).getModel() ), artifactResolver ), config );
 
@@ -152,8 +148,6 @@
             project.setFile( projectDescriptor );
 
             setBuildOutputDirectoryOnParent( project );
-
-        }
         return project;
     }
 
@@ -170,21 +164,11 @@
         return buildFromRepository( artifact, remoteArtifactRepositories, localRepository );
     }
 
-
     public MavenProject buildFromRepository( Artifact artifact, List remoteArtifactRepositories,
                                              ArtifactRepository localRepository )
         throws ProjectBuildingException
     {
-        MavenProject project = null;
-        if ( !Artifact.LATEST_VERSION.equals( artifact.getVersion() ) &&
-            !Artifact.RELEASE_VERSION.equals( artifact.getVersion() ) )
-        {
-            project =
-                projectWorkspace.getProject( artifact.getGroupId(), artifact.getArtifactId(), artifact.getVersion() );
-        }
         File f = artifact.getFile();
-        if ( project == null )
-        {
             repositoryHelper.findModelFromRepository( artifact, remoteArtifactRepositories, localRepository );
 
             ProjectBuilderConfiguration config =
@@ -195,10 +179,9 @@
             artifactRepositories.addAll( repositoryHelper.buildArtifactRepositories(
                 getSuperProject( config, artifact.getFile(), false ).getModel() ) );
 
-            project = readModelFromLocalPath( "unknown", artifact.getFile(), new PomArtifactResolver(
+            MavenProject project = readModelFromLocalPath( "unknown", artifact.getFile(), new PomArtifactResolver(
                 config.getLocalRepository(), artifactRepositories, artifactResolver ), config );
             project = buildInternal( project.getModel(), config, artifact.getFile(), project.getParentFile(), false );
-        }
 
         artifact.setFile( f );
         project.setVersion( artifact.getVersion() );
@@ -440,9 +423,6 @@
                                                                                externalProfileManager ) );
         project.setActiveProfiles( projectProfiles );
 
-        projectWorkspace.storeProjectByCoordinate( project );
-        projectWorkspace.storeProjectByFile( project );
-
         return project;
     }
 

Modified: maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java
URL: http://svn.apache.org/viewvc/maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java?rev=699773&r1=699772&r2=699773&view=diff
==============================================================================
--- maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java (original)
+++ maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java Sat Sep 27 21:52:53 2008
@@ -56,12 +56,7 @@
 import org.codehaus.plexus.util.StringUtils;
 
 import java.io.File;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.LinkedHashSet;
-import java.util.List;
-import java.util.Set;
+import java.util.*;
 
 /**
  * @author Jason van Zyl
@@ -149,6 +144,8 @@
         return artifact.getGroupId() + ":" + artifact.getArtifactId() + ":" + artifact.getVersion();
     }
 
+    private HashMap<String, MavenProject> hm = new HashMap<String, MavenProject>();
+    
     private ProjectRelocation retrieveRelocatedProject( Artifact artifact, ArtifactRepository localRepository,
                                                         List<ArtifactRepository> remoteRepositories )
         throws ArtifactMetadataRetrievalException
@@ -184,44 +181,55 @@
             }
             else
             {
-                try
+
+                if(hm.containsKey(pomArtifact.getId()))
                 {
-                    project =
-                        mavenProjectBuilder.buildFromRepository( pomArtifact, remoteRepositories, localRepository );
+                    project = hm.get(pomArtifact.getId());
                 }
-                catch ( InvalidProjectModelException e )
+                else
                 {
-                    handleInvalidOrMissingMavenPOM( artifact, e );
-
-                    if ( getLogger().isDebugEnabled() )
+                    try
                     {
-                        getLogger().debug( "Reason: " + e.getMessage() );
+                        project =
+                            mavenProjectBuilder.buildFromRepository( pomArtifact, remoteRepositories, localRepository );
+                        hm.put(pomArtifact.getId(), project);
 
-                        ModelValidationResult validationResult = e.getValidationResult();
+                    }
+                    catch ( InvalidProjectModelException e )
+                    {
+                        handleInvalidOrMissingMavenPOM( artifact, e );
 
-                        if ( validationResult != null )
+                        if ( getLogger().isDebugEnabled() )
                         {
-                            getLogger().debug( "\nValidation Errors:" );
-                            for ( Iterator i = validationResult.getMessages().iterator(); i.hasNext(); )
+                            getLogger().debug( "Reason: " + e.getMessage() );
+
+                            ModelValidationResult validationResult = e.getValidationResult();
+
+                            if ( validationResult != null )
                             {
-                                getLogger().debug( i.next().toString() );
+                                getLogger().debug( "\nValidation Errors:" );
+                                for ( Iterator i = validationResult.getMessages().iterator(); i.hasNext(); )
+                                {
+                                    getLogger().debug( i.next().toString() );
+                                }
+                                getLogger().debug( "\n" );
+                            }
+                            else
+                            {
+                                getLogger().debug( "", e );
                             }
-                            getLogger().debug( "\n" );
-                        }
-                        else
-                        {
-                            getLogger().debug( "", e );
                         }
+
+                        project = null;
                     }
+                    catch ( ProjectBuildingException e )
+                    {
+                        handleInvalidOrMissingMavenPOM( artifact, e );
 
-                    project = null;
+                        project = null;
+                    }
                 }
-                catch ( ProjectBuildingException e )
-                {
-                    handleInvalidOrMissingMavenPOM( artifact, e );
 
-                    project = null;
-                }
 
                 if ( project != null )
                 {



Re: svn commit: r699773 - in /maven/components/trunk/maven-project/src/main/java/org/apache/maven/project: DefaultMavenProjectBuilder.java artifact/MavenMetadataSource.java

Posted by Shane Isbell <sh...@gmail.com>.
On Mon, Sep 29, 2008 at 1:47 PM, Stephen Connolly <
stephen.alan.connolly@gmail.com> wrote:

> Can we not wrap them with unmodifyable wrappers?

It's easy to make the MavenProject/Model immutable, but it would break most
of Maven, which uses MavenProject as a value object, kicking it around and
taking whacks at updating it. We are getting there, but still a ways to go.

Shane

Re: svn commit: r699773 - in /maven/components/trunk/maven-project/src/main/java/org/apache/maven/project: DefaultMavenProjectBuilder.java artifact/MavenMetadataSource.java

Posted by Stephen Connolly <st...@gmail.com>.
Can we not wrap them with unmodifyable wrappers?

2008/9/29 Shane Isbell <sh...@gmail.com>

> One of the problems with any sort of caching right now is that MavenProject
> and Model are mutable. Passing around the same reference of
> these ubiquitous objects, means that any component can modify other
> components, changing their behavior internally. This is a pretty serious
> encapsulation issue. It's even worse that a class could invoke
> MavenProject.getModel and then leak the mutable model, which backs
> MavenProject, out to some other class.
> Having a global caching strategy without enforcing some level of
> immutability in these objects could exacerbate this encapsulation problem.
>
> We may consider having the embedders (or some attached component)
> explicitly control the updates and deletions of the cache. If mutability of
> MavenProject/Model is absolutely necessary, attaching listeners could also
> prove beneficial so that if there are changes to a cached object, at least
> the classes using the objects could react, rather than having their
> behavior
> invisibly changed.
>
> Shane
>
> On Mon, Sep 29, 2008 at 12:41 AM, Stephen Connolly <
> stephen.alan.connolly@gmail.com> wrote:
>
> > 2008/9/29 Milos Kleint <mk...@gmail.com>
> >
> > > more likely should be something that gets carried over with the
> > > request. That however goes against the component architecture a bit as
> > > it requires the context (request) to be carried along through all the
> > > components. AFAIK workspace attempted to do just that, but I never
> > > took a closer look.
> > >
> >
> > Yes, I agree... that would be the right way
> >
> > >
> > > weak references are rather unpredictable and will not help for
> > > concurrent processing in multiple threads.
> > >
> >
> > However, they could be used to help caching... if the weak references are
> > entirely managed in the caching layer then multiple threads would not be
> as
> > much of an issue. I was not suggesting keeping the weak references in
> each
> > plugin, rather wrap the builder in a cached builder that keeps weak
> > references... It would also have the side-effect of allowing Maven (from
> > the
> > command line) to trade memory for performance when memory is running
> tight.
> >
> > (But, yes,  in general they are a pane in the h*le)
> >
> > >
> > > Milos
> > >
> > > On Mon, Sep 29, 2008 at 8:08 AM, Stephen Connolly
> > > <st...@gmail.com> wrote:
> > > > weakreferences?
> > > >
> > > > 2008/9/29 Shane Isbell <sh...@gmail.com>
> > > >
> > > >> When Jason tested the removal of the workspace, which handles
> caching
> > of
> > > >> MavenProjects, it exposed a lot of bad behaviors within Maven, such
> > > >> multiple
> > > >> instances of ProjectBuilder, excessive numbers of calls to
> > > ProjectBuilder
> > > >> (54K in one build of trunk). We put back in some simple caching
> > > mechanisms
> > > >> (hash maps) to get the build back to an acceptable speed.
> > > >> Obviously, hash maps is not the solution for the embedder, as that
> > would
> > > be
> > > >> a memory leak and doesn't provide easy clearing. That's something we
> > > need
> > > >> to
> > > >> discuss on the list: how we should handle caching within Maven, as
> > well
> > > as
> > > >> reducing the number of calls to the builder.
> > > >>
> > > >> Thanks,
> > > >> Shane
> > > >>
> > > >> On Sun, Sep 28, 2008 at 5:40 PM, Brett Porter <br...@apache.org>
> > wrote:
> > > >>
> > > >> > I was about to ask exactly the same question, Milos beat me to it.
> > > >> >
> > > >> > Can you elaborate more please?
> > > >> >
> > > >> > Thanks,
> > > >> > Brett
> > > >> >
> > > >> > On 29/09/2008, at 8:12 AM, Jason van Zyl wrote:
> > > >> >
> > > >> >  We're just in the middle of ripping some stuff down and building
> it
> > > back
> > > >> >> up. All with the end of making it embedder friendly.
> > > >> >>
> > > >> >> On 28-Sep-08, at 2:50 PM, Milos Kleint wrote:
> > > >> >>
> > > >> >>  Hello Shane,
> > > >> >>>
> > > >> >>> How will the cache be cleared? Other than dumping and restarting
> > the
> > > >> >>> container?
> > > >> >>> That would be a problem for embedded project loading.
> > > >> >>>
> > > >> >>> Milos
> > > >> >>>
> > > >> >>>
> > > >> >>>
> > > >> > --
> > > >> > Brett Porter
> > > >> > brett@apache.org
> > > >> > http://blogs.exist.com/bporter/
> > > >> >
> > > >> >
> > > >> >
> > > >> >
> > ---------------------------------------------------------------------
> > > >> > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> > > >> > For additional commands, e-mail: dev-help@maven.apache.org
> > > >> >
> > > >> >
> > > >>
> > > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> > > For additional commands, e-mail: dev-help@maven.apache.org
> > >
> > >
> >
>

Re: svn commit: r699773 - in /maven/components/trunk/maven-project/src/main/java/org/apache/maven/project: DefaultMavenProjectBuilder.java artifact/MavenMetadataSource.java

Posted by Shane Isbell <sh...@gmail.com>.
One of the problems with any sort of caching right now is that MavenProject
and Model are mutable. Passing around the same reference of
these ubiquitous objects, means that any component can modify other
components, changing their behavior internally. This is a pretty serious
encapsulation issue. It's even worse that a class could invoke
MavenProject.getModel and then leak the mutable model, which backs
MavenProject, out to some other class.
Having a global caching strategy without enforcing some level of
immutability in these objects could exacerbate this encapsulation problem.

We may consider having the embedders (or some attached component)
explicitly control the updates and deletions of the cache. If mutability of
MavenProject/Model is absolutely necessary, attaching listeners could also
prove beneficial so that if there are changes to a cached object, at least
the classes using the objects could react, rather than having their behavior
invisibly changed.

Shane

On Mon, Sep 29, 2008 at 12:41 AM, Stephen Connolly <
stephen.alan.connolly@gmail.com> wrote:

> 2008/9/29 Milos Kleint <mk...@gmail.com>
>
> > more likely should be something that gets carried over with the
> > request. That however goes against the component architecture a bit as
> > it requires the context (request) to be carried along through all the
> > components. AFAIK workspace attempted to do just that, but I never
> > took a closer look.
> >
>
> Yes, I agree... that would be the right way
>
> >
> > weak references are rather unpredictable and will not help for
> > concurrent processing in multiple threads.
> >
>
> However, they could be used to help caching... if the weak references are
> entirely managed in the caching layer then multiple threads would not be as
> much of an issue. I was not suggesting keeping the weak references in each
> plugin, rather wrap the builder in a cached builder that keeps weak
> references... It would also have the side-effect of allowing Maven (from
> the
> command line) to trade memory for performance when memory is running tight.
>
> (But, yes,  in general they are a pane in the h*le)
>
> >
> > Milos
> >
> > On Mon, Sep 29, 2008 at 8:08 AM, Stephen Connolly
> > <st...@gmail.com> wrote:
> > > weakreferences?
> > >
> > > 2008/9/29 Shane Isbell <sh...@gmail.com>
> > >
> > >> When Jason tested the removal of the workspace, which handles caching
> of
> > >> MavenProjects, it exposed a lot of bad behaviors within Maven, such
> > >> multiple
> > >> instances of ProjectBuilder, excessive numbers of calls to
> > ProjectBuilder
> > >> (54K in one build of trunk). We put back in some simple caching
> > mechanisms
> > >> (hash maps) to get the build back to an acceptable speed.
> > >> Obviously, hash maps is not the solution for the embedder, as that
> would
> > be
> > >> a memory leak and doesn't provide easy clearing. That's something we
> > need
> > >> to
> > >> discuss on the list: how we should handle caching within Maven, as
> well
> > as
> > >> reducing the number of calls to the builder.
> > >>
> > >> Thanks,
> > >> Shane
> > >>
> > >> On Sun, Sep 28, 2008 at 5:40 PM, Brett Porter <br...@apache.org>
> wrote:
> > >>
> > >> > I was about to ask exactly the same question, Milos beat me to it.
> > >> >
> > >> > Can you elaborate more please?
> > >> >
> > >> > Thanks,
> > >> > Brett
> > >> >
> > >> > On 29/09/2008, at 8:12 AM, Jason van Zyl wrote:
> > >> >
> > >> >  We're just in the middle of ripping some stuff down and building it
> > back
> > >> >> up. All with the end of making it embedder friendly.
> > >> >>
> > >> >> On 28-Sep-08, at 2:50 PM, Milos Kleint wrote:
> > >> >>
> > >> >>  Hello Shane,
> > >> >>>
> > >> >>> How will the cache be cleared? Other than dumping and restarting
> the
> > >> >>> container?
> > >> >>> That would be a problem for embedded project loading.
> > >> >>>
> > >> >>> Milos
> > >> >>>
> > >> >>>
> > >> >>>
> > >> > --
> > >> > Brett Porter
> > >> > brett@apache.org
> > >> > http://blogs.exist.com/bporter/
> > >> >
> > >> >
> > >> >
> > >> >
> ---------------------------------------------------------------------
> > >> > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> > >> > For additional commands, e-mail: dev-help@maven.apache.org
> > >> >
> > >> >
> > >>
> > >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> > For additional commands, e-mail: dev-help@maven.apache.org
> >
> >
>

Re: svn commit: r699773 - in /maven/components/trunk/maven-project/src/main/java/org/apache/maven/project: DefaultMavenProjectBuilder.java artifact/MavenMetadataSource.java

Posted by Stephen Connolly <st...@gmail.com>.
2008/9/29 Milos Kleint <mk...@gmail.com>

> more likely should be something that gets carried over with the
> request. That however goes against the component architecture a bit as
> it requires the context (request) to be carried along through all the
> components. AFAIK workspace attempted to do just that, but I never
> took a closer look.
>

Yes, I agree... that would be the right way

>
> weak references are rather unpredictable and will not help for
> concurrent processing in multiple threads.
>

However, they could be used to help caching... if the weak references are
entirely managed in the caching layer then multiple threads would not be as
much of an issue. I was not suggesting keeping the weak references in each
plugin, rather wrap the builder in a cached builder that keeps weak
references... It would also have the side-effect of allowing Maven (from the
command line) to trade memory for performance when memory is running tight.

(But, yes,  in general they are a pane in the h*le)

>
> Milos
>
> On Mon, Sep 29, 2008 at 8:08 AM, Stephen Connolly
> <st...@gmail.com> wrote:
> > weakreferences?
> >
> > 2008/9/29 Shane Isbell <sh...@gmail.com>
> >
> >> When Jason tested the removal of the workspace, which handles caching of
> >> MavenProjects, it exposed a lot of bad behaviors within Maven, such
> >> multiple
> >> instances of ProjectBuilder, excessive numbers of calls to
> ProjectBuilder
> >> (54K in one build of trunk). We put back in some simple caching
> mechanisms
> >> (hash maps) to get the build back to an acceptable speed.
> >> Obviously, hash maps is not the solution for the embedder, as that would
> be
> >> a memory leak and doesn't provide easy clearing. That's something we
> need
> >> to
> >> discuss on the list: how we should handle caching within Maven, as well
> as
> >> reducing the number of calls to the builder.
> >>
> >> Thanks,
> >> Shane
> >>
> >> On Sun, Sep 28, 2008 at 5:40 PM, Brett Porter <br...@apache.org> wrote:
> >>
> >> > I was about to ask exactly the same question, Milos beat me to it.
> >> >
> >> > Can you elaborate more please?
> >> >
> >> > Thanks,
> >> > Brett
> >> >
> >> > On 29/09/2008, at 8:12 AM, Jason van Zyl wrote:
> >> >
> >> >  We're just in the middle of ripping some stuff down and building it
> back
> >> >> up. All with the end of making it embedder friendly.
> >> >>
> >> >> On 28-Sep-08, at 2:50 PM, Milos Kleint wrote:
> >> >>
> >> >>  Hello Shane,
> >> >>>
> >> >>> How will the cache be cleared? Other than dumping and restarting the
> >> >>> container?
> >> >>> That would be a problem for embedded project loading.
> >> >>>
> >> >>> Milos
> >> >>>
> >> >>>
> >> >>>
> >> > --
> >> > Brett Porter
> >> > brett@apache.org
> >> > http://blogs.exist.com/bporter/
> >> >
> >> >
> >> >
> >> > ---------------------------------------------------------------------
> >> > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> >> > For additional commands, e-mail: dev-help@maven.apache.org
> >> >
> >> >
> >>
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>

Re: svn commit: r699773 - in /maven/components/trunk/maven-project/src/main/java/org/apache/maven/project: DefaultMavenProjectBuilder.java artifact/MavenMetadataSource.java

Posted by Milos Kleint <mk...@gmail.com>.
more likely should be something that gets carried over with the
request. That however goes against the component architecture a bit as
it requires the context (request) to be carried along through all the
components. AFAIK workspace attempted to do just that, but I never
took a closer look.

weak references are rather unpredictable and will not help for
concurrent processing in multiple threads.

Milos

On Mon, Sep 29, 2008 at 8:08 AM, Stephen Connolly
<st...@gmail.com> wrote:
> weakreferences?
>
> 2008/9/29 Shane Isbell <sh...@gmail.com>
>
>> When Jason tested the removal of the workspace, which handles caching of
>> MavenProjects, it exposed a lot of bad behaviors within Maven, such
>> multiple
>> instances of ProjectBuilder, excessive numbers of calls to ProjectBuilder
>> (54K in one build of trunk). We put back in some simple caching mechanisms
>> (hash maps) to get the build back to an acceptable speed.
>> Obviously, hash maps is not the solution for the embedder, as that would be
>> a memory leak and doesn't provide easy clearing. That's something we need
>> to
>> discuss on the list: how we should handle caching within Maven, as well as
>> reducing the number of calls to the builder.
>>
>> Thanks,
>> Shane
>>
>> On Sun, Sep 28, 2008 at 5:40 PM, Brett Porter <br...@apache.org> wrote:
>>
>> > I was about to ask exactly the same question, Milos beat me to it.
>> >
>> > Can you elaborate more please?
>> >
>> > Thanks,
>> > Brett
>> >
>> > On 29/09/2008, at 8:12 AM, Jason van Zyl wrote:
>> >
>> >  We're just in the middle of ripping some stuff down and building it back
>> >> up. All with the end of making it embedder friendly.
>> >>
>> >> On 28-Sep-08, at 2:50 PM, Milos Kleint wrote:
>> >>
>> >>  Hello Shane,
>> >>>
>> >>> How will the cache be cleared? Other than dumping and restarting the
>> >>> container?
>> >>> That would be a problem for embedded project loading.
>> >>>
>> >>> Milos
>> >>>
>> >>>
>> >>>
>> > --
>> > Brett Porter
>> > brett@apache.org
>> > http://blogs.exist.com/bporter/
>> >
>> >
>> >
>> > ---------------------------------------------------------------------
>> > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> > For additional commands, e-mail: dev-help@maven.apache.org
>> >
>> >
>>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: svn commit: r699773 - in /maven/components/trunk/maven-project/src/main/java/org/apache/maven/project: DefaultMavenProjectBuilder.java artifact/MavenMetadataSource.java

Posted by Stephen Connolly <st...@gmail.com>.
weakreferences?

2008/9/29 Shane Isbell <sh...@gmail.com>

> When Jason tested the removal of the workspace, which handles caching of
> MavenProjects, it exposed a lot of bad behaviors within Maven, such
> multiple
> instances of ProjectBuilder, excessive numbers of calls to ProjectBuilder
> (54K in one build of trunk). We put back in some simple caching mechanisms
> (hash maps) to get the build back to an acceptable speed.
> Obviously, hash maps is not the solution for the embedder, as that would be
> a memory leak and doesn't provide easy clearing. That's something we need
> to
> discuss on the list: how we should handle caching within Maven, as well as
> reducing the number of calls to the builder.
>
> Thanks,
> Shane
>
> On Sun, Sep 28, 2008 at 5:40 PM, Brett Porter <br...@apache.org> wrote:
>
> > I was about to ask exactly the same question, Milos beat me to it.
> >
> > Can you elaborate more please?
> >
> > Thanks,
> > Brett
> >
> > On 29/09/2008, at 8:12 AM, Jason van Zyl wrote:
> >
> >  We're just in the middle of ripping some stuff down and building it back
> >> up. All with the end of making it embedder friendly.
> >>
> >> On 28-Sep-08, at 2:50 PM, Milos Kleint wrote:
> >>
> >>  Hello Shane,
> >>>
> >>> How will the cache be cleared? Other than dumping and restarting the
> >>> container?
> >>> That would be a problem for embedded project loading.
> >>>
> >>> Milos
> >>>
> >>>
> >>>
> > --
> > Brett Porter
> > brett@apache.org
> > http://blogs.exist.com/bporter/
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> > For additional commands, e-mail: dev-help@maven.apache.org
> >
> >
>

Re: svn commit: r699773 - in /maven/components/trunk/maven-project/src/main/java/org/apache/maven/project: DefaultMavenProjectBuilder.java artifact/MavenMetadataSource.java

Posted by Shane Isbell <sh...@gmail.com>.
When Jason tested the removal of the workspace, which handles caching of
MavenProjects, it exposed a lot of bad behaviors within Maven, such multiple
instances of ProjectBuilder, excessive numbers of calls to ProjectBuilder
(54K in one build of trunk). We put back in some simple caching mechanisms
(hash maps) to get the build back to an acceptable speed.
Obviously, hash maps is not the solution for the embedder, as that would be
a memory leak and doesn't provide easy clearing. That's something we need to
discuss on the list: how we should handle caching within Maven, as well as
reducing the number of calls to the builder.

Thanks,
Shane

On Sun, Sep 28, 2008 at 5:40 PM, Brett Porter <br...@apache.org> wrote:

> I was about to ask exactly the same question, Milos beat me to it.
>
> Can you elaborate more please?
>
> Thanks,
> Brett
>
> On 29/09/2008, at 8:12 AM, Jason van Zyl wrote:
>
>  We're just in the middle of ripping some stuff down and building it back
>> up. All with the end of making it embedder friendly.
>>
>> On 28-Sep-08, at 2:50 PM, Milos Kleint wrote:
>>
>>  Hello Shane,
>>>
>>> How will the cache be cleared? Other than dumping and restarting the
>>> container?
>>> That would be a problem for embedded project loading.
>>>
>>> Milos
>>>
>>>
>>>
> --
> Brett Porter
> brett@apache.org
> http://blogs.exist.com/bporter/
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>

Re: svn commit: r699773 - in /maven/components/trunk/maven-project/src/main/java/org/apache/maven/project: DefaultMavenProjectBuilder.java artifact/MavenMetadataSource.java

Posted by Brett Porter <br...@apache.org>.
I was about to ask exactly the same question, Milos beat me to it.

Can you elaborate more please?

Thanks,
Brett

On 29/09/2008, at 8:12 AM, Jason van Zyl wrote:

> We're just in the middle of ripping some stuff down and building it  
> back up. All with the end of making it embedder friendly.
>
> On 28-Sep-08, at 2:50 PM, Milos Kleint wrote:
>
>> Hello Shane,
>>
>> How will the cache be cleared? Other than dumping and restarting  
>> the container?
>> That would be a problem for embedded project loading.
>>
>> Milos
>>
>>

--
Brett Porter
brett@apache.org
http://blogs.exist.com/bporter/


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: svn commit: r699773 - in /maven/components/trunk/maven-project/src/main/java/org/apache/maven/project: DefaultMavenProjectBuilder.java artifact/MavenMetadataSource.java

Posted by Jason van Zyl <ja...@maven.org>.
We're just in the middle of ripping some stuff down and building it  
back up. All with the end of making it embedder friendly.

On 28-Sep-08, at 2:50 PM, Milos Kleint wrote:

> Hello Shane,
>
> How will the cache be cleared? Other than dumping and restarting the  
> container?
> That would be a problem for embedded project loading.
>
> Milos
>
> On Sun, Sep 28, 2008 at 6:52 AM,  <si...@apache.org> wrote:
>> Author: sisbell
>> Date: Sat Sep 27 21:52:53 2008
>> New Revision: 699773
>>
>> URL: http://svn.apache.org/viewvc?rev=699773&view=rev
>> Log:
>> Removed use of workspace from project builder. In the build of  
>> trunk, there were about 50K of calls from MavenMetadataSource to  
>> the project builder. I put a simple hashmap cache in the metadata  
>> source to reduce calls to dozens.
>>
>> Modified:
>>   maven/components/trunk/maven-project/src/main/java/org/apache/ 
>> maven/project/DefaultMavenProjectBuilder.java
>>   maven/components/trunk/maven-project/src/main/java/org/apache/ 
>> maven/project/artifact/MavenMetadataSource.java
>>
>> Modified: maven/components/trunk/maven-project/src/main/java/org/ 
>> apache/maven/project/DefaultMavenProjectBuilder.java
>> URL: http://svn.apache.org/viewvc/maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/DefaultMavenProjectBuilder.java?rev=699773&r1=699772&r2=699773&view=diff
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> =====================================================================
>> --- maven/components/trunk/maven-project/src/main/java/org/apache/ 
>> maven/project/DefaultMavenProjectBuilder.java (original)
>> +++ maven/components/trunk/maven-project/src/main/java/org/apache/ 
>> maven/project/DefaultMavenProjectBuilder.java Sat Sep 27 21:52:53  
>> 2008
>> @@ -132,11 +132,7 @@
>>    public MavenProject build( File projectDescriptor,  
>> ProjectBuilderConfiguration config )
>>        throws ProjectBuildingException
>>    {
>> -        MavenProject project =  
>> projectWorkspace.getProject( projectDescriptor );
>> -
>> -        if ( project == null )
>> -        {
>> -            project = readModelFromLocalPath( "unknown",  
>> projectDescriptor, new PomArtifactResolver(
>> +            MavenProject project =  
>> readModelFromLocalPath( "unknown", projectDescriptor, new  
>> PomArtifactResolver(
>>                config.getLocalRepository(),  
>> repositoryHelper.buildArtifactRepositories(
>>                getSuperProject( config, projectDescriptor,  
>> true ).getModel() ), artifactResolver ), config );
>>
>> @@ -152,8 +148,6 @@
>>            project.setFile( projectDescriptor );
>>
>>            setBuildOutputDirectoryOnParent( project );
>> -
>> -        }
>>        return project;
>>    }
>>
>> @@ -170,21 +164,11 @@
>>        return buildFromRepository( artifact,  
>> remoteArtifactRepositories, localRepository );
>>    }
>>
>> -
>>    public MavenProject buildFromRepository( Artifact artifact, List  
>> remoteArtifactRepositories,
>>                                             ArtifactRepository  
>> localRepository )
>>        throws ProjectBuildingException
>>    {
>> -        MavenProject project = null;
>> -        if ( ! 
>> Artifact.LATEST_VERSION.equals( artifact.getVersion() ) &&
>> -            ! 
>> Artifact.RELEASE_VERSION.equals( artifact.getVersion() ) )
>> -        {
>> -            project =
>> -                 
>> projectWorkspace.getProject( artifact.getGroupId(),  
>> artifact.getArtifactId(), artifact.getVersion() );
>> -        }
>>        File f = artifact.getFile();
>> -        if ( project == null )
>> -        {
>>            repositoryHelper.findModelFromRepository( artifact,  
>> remoteArtifactRepositories, localRepository );
>>
>>            ProjectBuilderConfiguration config =
>> @@ -195,10 +179,9 @@
>>             
>> artifactRepositories 
>> .addAll( repositoryHelper.buildArtifactRepositories(
>>                getSuperProject( config, artifact.getFile(),  
>> false ).getModel() ) );
>>
>> -            project = readModelFromLocalPath( "unknown",  
>> artifact.getFile(), new PomArtifactResolver(
>> +            MavenProject project =  
>> readModelFromLocalPath( "unknown", artifact.getFile(), new  
>> PomArtifactResolver(
>>                config.getLocalRepository(), artifactRepositories,  
>> artifactResolver ), config );
>>            project = buildInternal( project.getModel(), config,  
>> artifact.getFile(), project.getParentFile(), false );
>> -        }
>>
>>        artifact.setFile( f );
>>        project.setVersion( artifact.getVersion() );
>> @@ -440,9 +423,6 @@
>>                                                                               externalProfileManager 
>>  ) );
>>        project.setActiveProfiles( projectProfiles );
>>
>> -        projectWorkspace.storeProjectByCoordinate( project );
>> -        projectWorkspace.storeProjectByFile( project );
>> -
>>        return project;
>>    }
>>
>>
>> Modified: maven/components/trunk/maven-project/src/main/java/org/ 
>> apache/maven/project/artifact/MavenMetadataSource.java
>> URL: http://svn.apache.org/viewvc/maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java?rev=699773&r1=699772&r2=699773&view=diff
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> =====================================================================
>> --- maven/components/trunk/maven-project/src/main/java/org/apache/ 
>> maven/project/artifact/MavenMetadataSource.java (original)
>> +++ maven/components/trunk/maven-project/src/main/java/org/apache/ 
>> maven/project/artifact/MavenMetadataSource.java Sat Sep 27 21:52:53  
>> 2008
>> @@ -56,12 +56,7 @@
>> import org.codehaus.plexus.util.StringUtils;
>>
>> import java.io.File;
>> -import java.util.ArrayList;
>> -import java.util.Collections;
>> -import java.util.Iterator;
>> -import java.util.LinkedHashSet;
>> -import java.util.List;
>> -import java.util.Set;
>> +import java.util.*;
>>
>> /**
>> * @author Jason van Zyl
>> @@ -149,6 +144,8 @@
>>        return artifact.getGroupId() + ":" +  
>> artifact.getArtifactId() + ":" + artifact.getVersion();
>>    }
>>
>> +    private HashMap<String, MavenProject> hm = new HashMap<String,  
>> MavenProject>();
>> +
>>    private ProjectRelocation retrieveRelocatedProject( Artifact  
>> artifact, ArtifactRepository localRepository,
>>                                                         
>> List<ArtifactRepository> remoteRepositories )
>>        throws ArtifactMetadataRetrievalException
>> @@ -184,44 +181,55 @@
>>            }
>>            else
>>            {
>> -                try
>> +
>> +                if(hm.containsKey(pomArtifact.getId()))
>>                {
>> -                    project =
>> -                         
>> mavenProjectBuilder.buildFromRepository( pomArtifact,  
>> remoteRepositories, localRepository );
>> +                    project = hm.get(pomArtifact.getId());
>>                }
>> -                catch ( InvalidProjectModelException e )
>> +                else
>>                {
>> -                    handleInvalidOrMissingMavenPOM( artifact, e );
>> -
>> -                    if ( getLogger().isDebugEnabled() )
>> +                    try
>>                    {
>> -                        getLogger().debug( "Reason: " +  
>> e.getMessage() );
>> +                        project =
>> +                             
>> mavenProjectBuilder.buildFromRepository( pomArtifact,  
>> remoteRepositories, localRepository );
>> +                        hm.put(pomArtifact.getId(), project);
>>
>> -                        ModelValidationResult validationResult =  
>> e.getValidationResult();
>> +                    }
>> +                    catch ( InvalidProjectModelException e )
>> +                    {
>> +                        handleInvalidOrMissingMavenPOM( artifact,  
>> e );
>>
>> -                        if ( validationResult != null )
>> +                        if ( getLogger().isDebugEnabled() )
>>                        {
>> -                            getLogger().debug( "\nValidation  
>> Errors:" );
>> -                            for ( Iterator i =  
>> validationResult.getMessages().iterator(); i.hasNext(); )
>> +                            getLogger().debug( "Reason: " +  
>> e.getMessage() );
>> +
>> +                            ModelValidationResult validationResult  
>> = e.getValidationResult();
>> +
>> +                            if ( validationResult != null )
>>                            {
>> -                                 
>> getLogger().debug( i.next().toString() );
>> +                                getLogger().debug( "\nValidation  
>> Errors:" );
>> +                                for ( Iterator i =  
>> validationResult.getMessages().iterator(); i.hasNext(); )
>> +                                {
>> +                                     
>> getLogger().debug( i.next().toString() );
>> +                                }
>> +                                getLogger().debug( "\n" );
>> +                            }
>> +                            else
>> +                            {
>> +                                getLogger().debug( "", e );
>>                            }
>> -                            getLogger().debug( "\n" );
>> -                        }
>> -                        else
>> -                        {
>> -                            getLogger().debug( "", e );
>>                        }
>> +
>> +                        project = null;
>>                    }
>> +                    catch ( ProjectBuildingException e )
>> +                    {
>> +                        handleInvalidOrMissingMavenPOM( artifact,  
>> e );
>>
>> -                    project = null;
>> +                        project = null;
>> +                    }
>>                }
>> -                catch ( ProjectBuildingException e )
>> -                {
>> -                    handleInvalidOrMissingMavenPOM( artifact, e );
>>
>> -                    project = null;
>> -                }
>>
>>                if ( project != null )
>>                {
>>
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>

Thanks,

Jason

----------------------------------------------------------
Jason van Zyl
Founder,  Apache Maven
jason at sonatype dot com
----------------------------------------------------------

the course of true love never did run smooth ...

  -- Shakespeare


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: svn commit: r699773 - in /maven/components/trunk/maven-project/src/main/java/org/apache/maven/project: DefaultMavenProjectBuilder.java artifact/MavenMetadataSource.java

Posted by Milos Kleint <mk...@gmail.com>.
Hello Shane,

How will the cache be cleared? Other than dumping and restarting the container?
That would be a problem for embedded project loading.

Milos

On Sun, Sep 28, 2008 at 6:52 AM,  <si...@apache.org> wrote:
> Author: sisbell
> Date: Sat Sep 27 21:52:53 2008
> New Revision: 699773
>
> URL: http://svn.apache.org/viewvc?rev=699773&view=rev
> Log:
> Removed use of workspace from project builder. In the build of trunk, there were about 50K of calls from MavenMetadataSource to the project builder. I put a simple hashmap cache in the metadata source to reduce calls to dozens.
>
> Modified:
>    maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/DefaultMavenProjectBuilder.java
>    maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java
>
> Modified: maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/DefaultMavenProjectBuilder.java
> URL: http://svn.apache.org/viewvc/maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/DefaultMavenProjectBuilder.java?rev=699773&r1=699772&r2=699773&view=diff
> ==============================================================================
> --- maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/DefaultMavenProjectBuilder.java (original)
> +++ maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/DefaultMavenProjectBuilder.java Sat Sep 27 21:52:53 2008
> @@ -132,11 +132,7 @@
>     public MavenProject build( File projectDescriptor, ProjectBuilderConfiguration config )
>         throws ProjectBuildingException
>     {
> -        MavenProject project = projectWorkspace.getProject( projectDescriptor );
> -
> -        if ( project == null )
> -        {
> -            project = readModelFromLocalPath( "unknown", projectDescriptor, new PomArtifactResolver(
> +            MavenProject project = readModelFromLocalPath( "unknown", projectDescriptor, new PomArtifactResolver(
>                 config.getLocalRepository(), repositoryHelper.buildArtifactRepositories(
>                 getSuperProject( config, projectDescriptor, true ).getModel() ), artifactResolver ), config );
>
> @@ -152,8 +148,6 @@
>             project.setFile( projectDescriptor );
>
>             setBuildOutputDirectoryOnParent( project );
> -
> -        }
>         return project;
>     }
>
> @@ -170,21 +164,11 @@
>         return buildFromRepository( artifact, remoteArtifactRepositories, localRepository );
>     }
>
> -
>     public MavenProject buildFromRepository( Artifact artifact, List remoteArtifactRepositories,
>                                              ArtifactRepository localRepository )
>         throws ProjectBuildingException
>     {
> -        MavenProject project = null;
> -        if ( !Artifact.LATEST_VERSION.equals( artifact.getVersion() ) &&
> -            !Artifact.RELEASE_VERSION.equals( artifact.getVersion() ) )
> -        {
> -            project =
> -                projectWorkspace.getProject( artifact.getGroupId(), artifact.getArtifactId(), artifact.getVersion() );
> -        }
>         File f = artifact.getFile();
> -        if ( project == null )
> -        {
>             repositoryHelper.findModelFromRepository( artifact, remoteArtifactRepositories, localRepository );
>
>             ProjectBuilderConfiguration config =
> @@ -195,10 +179,9 @@
>             artifactRepositories.addAll( repositoryHelper.buildArtifactRepositories(
>                 getSuperProject( config, artifact.getFile(), false ).getModel() ) );
>
> -            project = readModelFromLocalPath( "unknown", artifact.getFile(), new PomArtifactResolver(
> +            MavenProject project = readModelFromLocalPath( "unknown", artifact.getFile(), new PomArtifactResolver(
>                 config.getLocalRepository(), artifactRepositories, artifactResolver ), config );
>             project = buildInternal( project.getModel(), config, artifact.getFile(), project.getParentFile(), false );
> -        }
>
>         artifact.setFile( f );
>         project.setVersion( artifact.getVersion() );
> @@ -440,9 +423,6 @@
>                                                                                externalProfileManager ) );
>         project.setActiveProfiles( projectProfiles );
>
> -        projectWorkspace.storeProjectByCoordinate( project );
> -        projectWorkspace.storeProjectByFile( project );
> -
>         return project;
>     }
>
>
> Modified: maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java
> URL: http://svn.apache.org/viewvc/maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java?rev=699773&r1=699772&r2=699773&view=diff
> ==============================================================================
> --- maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java (original)
> +++ maven/components/trunk/maven-project/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java Sat Sep 27 21:52:53 2008
> @@ -56,12 +56,7 @@
>  import org.codehaus.plexus.util.StringUtils;
>
>  import java.io.File;
> -import java.util.ArrayList;
> -import java.util.Collections;
> -import java.util.Iterator;
> -import java.util.LinkedHashSet;
> -import java.util.List;
> -import java.util.Set;
> +import java.util.*;
>
>  /**
>  * @author Jason van Zyl
> @@ -149,6 +144,8 @@
>         return artifact.getGroupId() + ":" + artifact.getArtifactId() + ":" + artifact.getVersion();
>     }
>
> +    private HashMap<String, MavenProject> hm = new HashMap<String, MavenProject>();
> +
>     private ProjectRelocation retrieveRelocatedProject( Artifact artifact, ArtifactRepository localRepository,
>                                                         List<ArtifactRepository> remoteRepositories )
>         throws ArtifactMetadataRetrievalException
> @@ -184,44 +181,55 @@
>             }
>             else
>             {
> -                try
> +
> +                if(hm.containsKey(pomArtifact.getId()))
>                 {
> -                    project =
> -                        mavenProjectBuilder.buildFromRepository( pomArtifact, remoteRepositories, localRepository );
> +                    project = hm.get(pomArtifact.getId());
>                 }
> -                catch ( InvalidProjectModelException e )
> +                else
>                 {
> -                    handleInvalidOrMissingMavenPOM( artifact, e );
> -
> -                    if ( getLogger().isDebugEnabled() )
> +                    try
>                     {
> -                        getLogger().debug( "Reason: " + e.getMessage() );
> +                        project =
> +                            mavenProjectBuilder.buildFromRepository( pomArtifact, remoteRepositories, localRepository );
> +                        hm.put(pomArtifact.getId(), project);
>
> -                        ModelValidationResult validationResult = e.getValidationResult();
> +                    }
> +                    catch ( InvalidProjectModelException e )
> +                    {
> +                        handleInvalidOrMissingMavenPOM( artifact, e );
>
> -                        if ( validationResult != null )
> +                        if ( getLogger().isDebugEnabled() )
>                         {
> -                            getLogger().debug( "\nValidation Errors:" );
> -                            for ( Iterator i = validationResult.getMessages().iterator(); i.hasNext(); )
> +                            getLogger().debug( "Reason: " + e.getMessage() );
> +
> +                            ModelValidationResult validationResult = e.getValidationResult();
> +
> +                            if ( validationResult != null )
>                             {
> -                                getLogger().debug( i.next().toString() );
> +                                getLogger().debug( "\nValidation Errors:" );
> +                                for ( Iterator i = validationResult.getMessages().iterator(); i.hasNext(); )
> +                                {
> +                                    getLogger().debug( i.next().toString() );
> +                                }
> +                                getLogger().debug( "\n" );
> +                            }
> +                            else
> +                            {
> +                                getLogger().debug( "", e );
>                             }
> -                            getLogger().debug( "\n" );
> -                        }
> -                        else
> -                        {
> -                            getLogger().debug( "", e );
>                         }
> +
> +                        project = null;
>                     }
> +                    catch ( ProjectBuildingException e )
> +                    {
> +                        handleInvalidOrMissingMavenPOM( artifact, e );
>
> -                    project = null;
> +                        project = null;
> +                    }
>                 }
> -                catch ( ProjectBuildingException e )
> -                {
> -                    handleInvalidOrMissingMavenPOM( artifact, e );
>
> -                    project = null;
> -                }
>
>                 if ( project != null )
>                 {
>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org