Re: [m2e-dev] M2Eclipse Performance (Was: [m2e-users] Having a Bad Week

[Igor Fedorenko <igor@xxxxxxxxxxxxxx>]

Heh. I am actually reworking SNAPSHOT resolution. Most likely m2e will only reresolve snapshots when explicitly asked to. Still, this does not eliminate dependency resolution, it may still be required after update from scm for example. Automatic dependency download is fundamental maven feature. I don't think we should try to suppress it.

Snjezana Peco <snjezana.peco@xxxxxxxxxx> wrote:

>Igor,
>
>Does m2eclipse re-resolve all SNAPSHOT dependencies when the Maven 
>Offline preference is checked?
>If m2eclipse doesn't do that, users could check the preference and 
>re-resolve SNAPSHOT dependencies manually using the m2eclipse's Run 
>As>Maven... command or Maven CLI.
>
>Regards,
>Snjeza
>
>Igor Fedorenko wrote:
>> Dependency resolution is not a special case. By default maven (and
>hence
>> m2e) re-resolve all SNAPSHOT dependencies every 24 fours, so projects
>> with long SNAPSHOT dependency trail will take very long time to
>refresh
>> every 24 hours.
>>
>> To be clear, I am not saying "no" to your proposal, I am just
>explaining
>> the current expected behaviour and reasons behind it. Please submit
>your
>> patches to bugzilla along with before/after performance numbers and
>lets
>> have more specific discussion on pros/cons of each of these items.
>>
>> -- 
>> Regards,
>> Igor
>>
>> On 11-04-12 11:30 AM, Snjezana Peco wrote:
>>> That is just a special case happening when importing a project the
>first
>>> time. The problem isn't in building a project, but in downloading
>Maven
>>> artifacts.
>>> Since we can't control downloading, we can ignore this case. The
>most of
>>> the users expect importing a project the first time to take a long
>time.
>>> Downloading artifacts is also slow when using IntelliJ IDEA.
>However, we
>>> could improve building a project when all the artifacts are
>downloaded -
>>> building a project after killing Eclipse, for instance. See
>>> https://bugs.eclipse.org/bugs/show_bug.cgi?id=342128#c3
>>>
>>> Snjeza
>>>
>>> Igor Fedorenko wrote:
>>>> clean local repo, slow/congested internal connection, jboss-as or
>any
>>>> other project with large dependency trail.
>>>>
>>>> -- 
>>>> Regards,
>>>> Igor
>>>>
>>>> On 11-04-12 10:02 AM, Snjezana Peco wrote:
>>>>> I have solved all the described issues (except #10 that I haven't 
>>>>> fixed
>>>>> yet) to one branch. Will try to separate them and analyze them
>>>>> individually. Could you point me to any project which build takes
>more
>>>>> than 10 min?
>>>>>
>>>>> Snjeza
>>>>>
>>>>> Igor Fedorenko wrote:
>>>>>> Thank you for doing this analysis, Snjezana. I'll comment on 
>>>>>> individual
>>>>>> items inline, but generally please submit git-format-patch
>patches 
>>>>>> for
>>>>>> separate problems as separate bugzillas. For each bugzilla/patch 
>>>>>> please
>>>>>> provide scenario that demonstrates the problem and before/after
>>>>>> performance metrics. Where practical, please provide
>corresponding
>>>>>> test(s) as a patch against m2e-core-test.
>>>>>>
>>>>>> -- 
>>>>>> Regards,
>>>>>> Igor
>>>>>>
>>>>>> On 11-04-11 07:59 PM, Snjezana Peco wrote:
>>>>>>> I have tested m2eclipse performance.
>>>>>>> These are my first impressions:
>>>>>>>
>>>>>>> 1) ProjectRegistryRefreshJob changes a workspace and fires a lot
>of
>>>>>>> resource change listeners.
>>>>>>> When creating a marker, for instance, this job fires five
>resource
>>>>>>> change listeners (createMarker and four the setAttribute
>methods).
>>>>>>> This job should be WorkspaceJob.
>>>>>>>
>>>>>>
>>>>>> Workspace project "refresh" is a two-stage process. First, m2e 
>>>>>> resolves
>>>>>> dependencies of all affected workspace projects. Depending on
>network
>>>>>> connection speed and number/size of dependencies, this can take
>>>>>> arbitrary long time (I saw builds that took well over 30 minutes)
>so
>>>>>> m2e
>>>>>> does not acquire any workspace locks during this stage. After all
>>>>>> dependencies are resolved, m2e acquires workspace lock and
>broadcasts
>>>>>> MavenProjectChangeEvents to all registered listeners. m2e.jdt,
>one of
>>>>>> the listeners, updates classpath and triggers (re)build of
>affected
>>>>>> projects upon reception of MavenProjectChangeEvents.
>>>>>>
>>>>>> Frankly, I am not convinced we need to keep workspace lock for
>>>>>> entire time of project refresh but I am open for discussion. To 
>>>>>> help me
>>>>>> understand pros and cons better
>>>>>>
>>>>>> 1a) Is marker creation the only cause of resource change events
>>>>>> fired by
>>>>>> the job or there are other sources?
>>>>>>
>>>>>> 1b) If marker creation is the only cause, what is the real 
>>>>>> overhead of
>>>>>> firing them as separate events vs one bulk event?
>>>>>>
>>>>>> Depending on answers to these two questions, delaying marker
>creation
>>>>>> until the second stage of refresh may be a better solution.
>>>>>>
>>>>>>> 2) ProjectRegistryRefreshJob and MavenBuilder (a builder job)
>aren't
>>>>>>> synchronized what often causes
>StaleMutableProjectRegistryException
>>>>>>> (ProjectRegistryManager, line 319) and that is the cause of
>starting
>>>>>>> this job again.
>>>>>>
>>>>>> I need to think some more about this, it looks like can disable
>>>>>> ProjectRegistryRefreshJob altogether when workspace autobuild is
>>>>>> enabled...
>>>>>>
>>>>>> We still need to run the job when autobuild is off, but this
>should
>>>>>> only
>>>>>> cause StaleMutableProjectRegistryException if the user manually
>runs
>>>>>> the
>>>>>> build when refresh job is running, so should not be a big deal
>>>>>>
>>>>>>> 3) LifecycleMappingFactory.getBundleMetadataSources is too often
>>>>>>> called.
>>>>>>> Every time it is called it reads the extension point registry
>and
>>>>>>> lifecycle mapping xml files. Caching would speed up the overall
>>>>>>> performance.
>>>>>>
>>>>>> We *guessed* that proper cache implementation, which deals with 
>>>>>> dynamic
>>>>>> bundle installation/uninstallation, is not worth the performance
>>>>>> benefits, but we did not actually measure it. Please provide
>>>>>> before/after numbers that demonstrate performance benefits,
>proper
>>>>>> cache
>>>>>> implementation with adequate test coverage and I will be happy to
>>>>>> review
>>>>>> and merge this change (and admit we guessed wrong ;-) ).
>>>>>>
>>>>>>> 4) The BuildPathManager.configureAttchedSourcesAndJavadoc method
>is
>>>>>>> very
>>>>>>> slow. I have improved it a little (DownloadSourcesJob isn't
>started
>>>>>>> when
>>>>>>> the download preferences are off), but it is possible to improve
>it
>>>>>>> more.
>>>>>>
>>>>>> DownloadSourcesJob should not be started if download sources and
>>>>>> javadoc
>>>>>> are disabled in maven configuration (see 
>>>>>> #getAttachedSourcesAndJavadoc
>>>>>> implementation and how its return value is used in
>scheduleDownload
>>>>>> around line 817). If you see DownloadSourcesJob start when
>>>>>> source/javadoc download is off, please provide a patch and
>>>>>> corresponding
>>>>>> unit test(s) and I will review and merge it.
>>>>>>
>>>>>> As for BuildPathManager.configureAttchedSourcesAndJavadoc
>>>>>> performance in
>>>>>> general, I briefly looked at it about a month ago. At that time 
>>>>>> Yourkit
>>>>>> was telling me that MavenImpl.getSettings was the problem, but I
>did
>>>>>> not
>>>>>> investigate it further. Do you know what makes
>>>>>> #configureAttchedSourcesAndJavadoc slow in your case?
>>>>>>
>>>>>>>
>>>>>>> 5) NexusIndexManager.mavenProjectChanged(...) always removes and
>
>>>>>>> adds
>>>>>>> back an artifact to the index repository. Performance will be
>much
>>>>>>> better if the index repository is updated only when the artifact
>>>>>>> doesn't
>>>>>>> exist or is changed. Searching the index repository is much
>faster
>>>>>>> than
>>>>>>> removing and adding artifacts.
>>>>>>
>>>>>> Please provide a patch with before/after performance numbers and
>I
>>>>>> will review and merge it.
>>>>>>
>>>>>>> 6) MarkerLocationService.addEditorHintMarkers calls two methods
>that
>>>>>>> acquire the WTP's IDOMModel
>>>>>>> (StructuredModelManager.getModelManager().getModelForRead(...))
>and
>>>>>>> release it. WTP's methods can be slow for larger files and it 
>>>>>>> would be
>>>>>>> better to acquire/release the model once for
>>>>>>> MarkerLocationService.addEditorHintMarkers.
>>>>>>> MarkerLocationService.addEditorHintMarkers is used only once.
>>>>>>>
>>>>>>
>>>>>> Please provide a patch with before/after performance numbers and
>I
>>>>>> will review and merge it.
>>>>>>
>>>>>>> 7) ProjectRegistryManager.applyMutableProjectRegistry calls
>>>>>>> stateReader.writeWorkspaceState(projectRegistry) when refreshing
>a
>>>>>>> project. Since the workspaceState.ser file is only used when
>>>>>>> starting a
>>>>>>> workspace, this method can be used only when stopping the
>>>>>>> org.eclipse.m2e.core bundle
>>>>>>
>>>>>> Please provide a patch with before/after performance numbers and
>I
>>>>>> will review and merge it.
>>>>>>
>>>>>>> 8) BuildPathManager.mavenProjectChanged updates Maven classpath
>>>>>>> containers of all projects no matter they are changed or not.
>They
>>>>>>> should be updated only for relevant events (event.getFlags() !=
>0).
>>>>>>
>>>>>> Please provide a patch with before/after performance numbers and
>I
>>>>>> will review and merge it.
>>>>>>
>>>>>>> 9) ProjectRegistryManager.refresh(MutableProjectRegistry
>newSt
-- 
Sent from my SGS