You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by Kristian Rosenvold <kr...@gmail.com> on 2012/07/26 14:44:48 UTC
Use case for funky class realm construction logic
There is a race condition in parallel builds that occurs related to
this piece of code:
http://maven.apache.org/ref/3.0.4/maven-core/xref/org/apache/maven/classrealm/DefaultClassRealmManager.html#75
The thing is, for some reason, there's a loop that retries the class
realm generation with a random suffix if the class realm already
exists. In a parallel run, there will be multiple threads requesting
the same realm-id, which semantically should map to the same instance
of the class realm.
(Most plugins do not really mind if there's a duplicate class realm
every now and then, but some take it very seriously ;)
I'm tempted to change the semantics of the "newRealm" method to
"getOrCreateRealm", since that seems to be the correct semantics no
matter what. I've tried tracking the origin of the while loop, and it
seems to be very old. Anyone have any idea of what purpose it served ?
Kristian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: Use case for funky class realm construction logic
Posted by Jason van Zyl <ja...@tesla.io>.
Kristian, you have the project handy that was causing the problem?
On Jul 26, 2012, at 12:54 PM, Igor Fedorenko wrote:
> I have not looked at parallel execution closely and don't fully
> understand the usecase, but I am not sure race condition is the code in
> DefaultClassRealmManager.
>
> As far as I can tell, DefaultClassRealmManager only provides create*
> methods, which create new realms and the current implementation looks
> proper.
>
> If parallel execution logic needs to be able to create-or-query plugin
> realms in thread-safe manner, I think this should be implemented
> somewhere at higher levels, where one-plugin-realm-per-plugin-perproject
> constraint can be enforced properly.
>
> Again, I do not know parallel execution, so I can be completely wrong
> about this.
>
> --
> Regards,
> Igor
>
> On 12-07-26 12:36 PM, Jason van Zyl wrote:
>> In that case we may just want to differentiate because the parallel execution use case is not one that happens inside the IDE. In the case of CLI execution it can all be handled up front as additional realm construction beyond the CLI execution isn't going to happen. In an IDE or shell wouldn't a map of realms keyed by GAV suffice? Sync on write everyone is free to read?
>>
>> On Jul 26, 2012, at 12:06 PM, Igor Fedorenko wrote:
>>
>>> This is required in m2e where multiple versions of the same plugin can
>>> coexist for long time in the same container.
>>>
>>> --
>>> Regards,
>>> Igor
>>>
>>> On 12-07-26 8:44 AM, Kristian Rosenvold wrote:
>>>> There is a race condition in parallel builds that occurs related to
>>>> this piece of code:
>>>>
>>>> http://maven.apache.org/ref/3.0.4/maven-core/xref/org/apache/maven/classrealm/DefaultClassRealmManager.html#75
>>>>
>>>> The thing is, for some reason, there's a loop that retries the class
>>>> realm generation with a random suffix if the class realm already
>>>> exists. In a parallel run, there will be multiple threads requesting
>>>> the same realm-id, which semantically should map to the same instance
>>>> of the class realm.
>>>>
>>>> (Most plugins do not really mind if there's a duplicate class realm
>>>> every now and then, but some take it very seriously ;)
>>>>
>>>> I'm tempted to change the semantics of the "newRealm" method to
>>>> "getOrCreateRealm", since that seems to be the correct semantics no
>>>> matter what. I've tried tracking the origin of the while loop, and it
>>>> seems to be very old. Anyone have any idea of what purpose it served ?
>>>>
>>>> Kristian
>>>>
>>>> ---------------------------------------------------------------------
>>>> 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
>>>
>>
>> Thanks,
>>
>> Jason
>>
>> ----------------------------------------------------------
>> Jason van Zyl
>> Founder & CTO, Sonatype
>> Founder, Apache Maven
>> http://twitter.com/jvanzyl
>> ---------------------------------------------------------
>>
>> The modern conservative is engaged in one of man's oldest exercises in moral philosophy; that is,
>> the search for a superior moral justification for selfishness.
>>
>> -- John Kenneth Galbraith
>>
>>
>>
>>
>>
>>
>
> ---------------------------------------------------------------------
> 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 & CTO, Sonatype
Founder, Apache Maven
http://twitter.com/jvanzyl
---------------------------------------------------------
A man enjoys his work when he understands the whole and when he
is responsible for the quality of the whole
-- Christopher Alexander, A Pattern Language
Re: Use case for funky class realm construction logic
Posted by Igor Fedorenko <ig...@ifedorenko.com>.
On 12-07-27 7:35 AM, Hervé BOUTEMY wrote:
> notice that a project can inject dependencies to a plugin: this is not so
> frequent, so reusing the same realm for each use of a plugin should often be
> ok, but not when specific dependencies are injected IMHO
>
This will be just one problem to solve if somebody decided to redesign
classloading. Build extensions, which are project specific but need to
be visible to plugin classloaders, is another problem. I also would not
be too surprised that some plugins actually assume per-project
classloder and use static variables to retain state. There are likely
more "interesting" cases I don't remember.
> but what about simply synchronizing the getPluginRealm method? It is not used
> so often IMHO, then shouldn't have much performance impact.
>
Yes, this is my suggestion.
--
Regards,
Igor
> Regards,
>
> Hervé
>
> Le vendredi 27 juillet 2012 06:55:51 Igor Fedorenko a écrit :
>> Shouldn't DefaultBuildPluginManager#getPluginRealm be synchronized?
>>
>> If two threads arrive at DefaultBuildPluginManager line 170 at about the
>> same time, they both will see null pluginRealm and both will then call
>> setupPluginRealm, which I believe will likely result in the two threads
>> using two different classloaders.
>>
>> Also, I still do not believe there is anything wrong with
>> design/implementation of DefaultClassRealmManager. If it is asked to
>> createPluginRealm multiple times, it creates multiple realms. Doing
>> otherwise will result in the same realm being reused for dirrefent
>> project/plugin combos and will mess up classloading unless entire
>> classloading approach is reworked.
>>
>> https://cwiki.apache.org/MAVEN/maven-3x-class-loading.html
>>
>> --
>> Regards,
>> Igor
>>
>> On 12-07-26 1:27 PM, Kristian Rosenvold wrote:
>>> I have some very specific logs in an issue
>>> (http://code.google.com/p/maven-svn-revision-number-plugin/issues/detail?i
>>> d=16#c85) indicating that to class realms are being created for the same
>>> plugin when there is only one execution.
>>>
>>> The behaviour seems totally consistent with hitting the catch block at
>>> line 100 of DefaultClassRealmManager, down to
>>> the suffix of the second class realm. Logically, this will happen at
>>> any time when the FIRST construction of the class realm happens in two
>>> parallel modules in the build. Imagine that you have two modules at
>>> the end of the build, both initializing the war plugin. If they are
>>> siblings in the parallel scheduling, the likelyhood of these two
>>> requesting realm in parallel is extremely high.
>>>
>>> The thread scheduling will ensure that two such modules will arrive at
>>> DefaultBuildPluginManager line 170 at more or less the same instant.
>>> Both will go through the if test, since this is the first time the
>>> realm is created. The interesting thing is that it seems to me like
>>> the cache key at line 304 must somehow be different, since the
>>> setupPluginRealm method is synchronized.
>>>
>>> It seems like I need to follow up on why the cache key is different,
>>> because technically it looks like we should be avoiding the second
>>> call to newRealm, meaning I should really be able to leave the funky
>>> logic as-is.
>>>
>>> WDYT ?
>>>
>>> Kristian
>>>
>>> 2012/7/26 Igor Fedorenko <ig...@ifedorenko.com>:
>>>> I have not looked at parallel execution closely and don't fully
>>>> understand the usecase, but I am not sure race condition is the code in
>>>> DefaultClassRealmManager.
>>>>
>>>> As far as I can tell, DefaultClassRealmManager only provides create*ock
>>>> methods, which create new realms and the current implementation looks
>>>> proper.
>>>>
>>>> If parallel execution logic needs to be able to create-or-query plugin
>>>> realms in thread-safe manner, I think this should be implemented
>>>> somewhere at higher levels, where one-plugin-realm-per-plugin-perproject
>>>> constraint can be enforced properly.
>>>>
>>>> Again, I do not know parallel execution, so I can be completely wrong
>>>> about this.
>>>>
>>>> --
>>>> Regards,
>>>> Igor
>>>>
>>>> On 12-07-26 12:36 PM, Jason van Zyl wrote:
>>>>> In that case we may just want to differentiate because the parallel
>>>>> execution use case is not one that happens inside the IDE. In the case
>>>>> of
>>>>> CLI execution it can all be handled up front as additional realm
>>>>> construction beyond the CLI execution isn't going to happen. In an IDE
>>>>> or
>>>>> shell wouldn't a map of realms keyed by GAV suffice? Sync on write
>>>>> everyone
>>>>> is free to read?
>>>>>
>>>>> On Jul 26, 2012, at 12:06 PM, Igor Fedorenko wrote:
>>>>>> This is required in m2e where multiple versions of the same plugin can
>>>>>> coexist for long time in the same container.
>>>>>>
>>>>>> --
>>>>>> Regards,
>>>>>> Igor
>>>>>>
>>>>>> On 12-07-26 8:44 AM, Kristian Rosenvold wrote:
>>>>>>> There is a race condition in parallel builds that occurs related to
>>>>>>> this piece of code:
>>>>>>>
>>>>>>>
>>>>>>> http://maven.apache.org/ref/3.0.4/maven-core/xref/org/apache/maven/cla
>>>>>>> ssrealm/DefaultClassRealmManager.html#75
>>>>>>>
>>>>>>> The thing is, for some reason, there's a loop that retries the class
>>>>>>> realm generation with a random suffix if the class realm already
>>>>>>> exists. In a parallel run, there will be multiple threads requesting
>>>>>>> the same realm-id, which semantically should map to the same instance
>>>>>>> of the class realm.
>>>>>>>
>>>>>>> (Most plugins do not really mind if there's a duplicate class realm
>>>>>>> every now and then, but some take it very seriously ;)
>>>>>>>
>>>>>>> I'm tempted to change the semantics of the "newRealm" method to
>>>>>>> "getOrCreateRealm", since that seems to be the correct semantics no
>>>>>>> matter what. I've tried tracking the origin of the while loop, and it
>>>>>>> seems to be very old. Anyone have any idea of what purpose it served ?
>>>>>>>
>>>>>>> Kristian
>>>>>>>
>>>>>>> ---------------------------------------------------------------------
>>>>>>> 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
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Jason
>>>>>
>>>>> ----------------------------------------------------------
>>>>> Jason van Zyl
>>>>> Founder & CTO, Sonatype
>>>>> Founder, Apache Maven
>>>>> http://twitter.com/jvanzyl
>>>>> ---------------------------------------------------------
>>>>>
>>>>> The modern conservative is engaged in one of man's oldest exercises in
>>>>> moral philosophy; that is,
>>>>> the search for a superior moral justification for selfishness.
>>>>>
>>>>> -- John Kenneth Galbraith
>>>>
>>>> ---------------------------------------------------------------------
>>>> 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
>>
>> ---------------------------------------------------------------------
>> 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
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: Use case for funky class realm construction logic
Posted by Hervé BOUTEMY <he...@free.fr>.
notice that a project can inject dependencies to a plugin: this is not so
frequent, so reusing the same realm for each use of a plugin should often be
ok, but not when specific dependencies are injected IMHO
but what about simply synchronizing the getPluginRealm method? It is not used
so often IMHO, then shouldn't have much performance impact.
Regards,
Hervé
Le vendredi 27 juillet 2012 06:55:51 Igor Fedorenko a écrit :
> Shouldn't DefaultBuildPluginManager#getPluginRealm be synchronized?
>
> If two threads arrive at DefaultBuildPluginManager line 170 at about the
> same time, they both will see null pluginRealm and both will then call
> setupPluginRealm, which I believe will likely result in the two threads
> using two different classloaders.
>
> Also, I still do not believe there is anything wrong with
> design/implementation of DefaultClassRealmManager. If it is asked to
> createPluginRealm multiple times, it creates multiple realms. Doing
> otherwise will result in the same realm being reused for dirrefent
> project/plugin combos and will mess up classloading unless entire
> classloading approach is reworked.
>
> https://cwiki.apache.org/MAVEN/maven-3x-class-loading.html
>
> --
> Regards,
> Igor
>
> On 12-07-26 1:27 PM, Kristian Rosenvold wrote:
> > I have some very specific logs in an issue
> > (http://code.google.com/p/maven-svn-revision-number-plugin/issues/detail?i
> > d=16#c85) indicating that to class realms are being created for the same
> > plugin when there is only one execution.
> >
> > The behaviour seems totally consistent with hitting the catch block at
> > line 100 of DefaultClassRealmManager, down to
> > the suffix of the second class realm. Logically, this will happen at
> > any time when the FIRST construction of the class realm happens in two
> > parallel modules in the build. Imagine that you have two modules at
> > the end of the build, both initializing the war plugin. If they are
> > siblings in the parallel scheduling, the likelyhood of these two
> > requesting realm in parallel is extremely high.
> >
> > The thread scheduling will ensure that two such modules will arrive at
> > DefaultBuildPluginManager line 170 at more or less the same instant.
> > Both will go through the if test, since this is the first time the
> > realm is created. The interesting thing is that it seems to me like
> > the cache key at line 304 must somehow be different, since the
> > setupPluginRealm method is synchronized.
> >
> > It seems like I need to follow up on why the cache key is different,
> > because technically it looks like we should be avoiding the second
> > call to newRealm, meaning I should really be able to leave the funky
> > logic as-is.
> >
> > WDYT ?
> >
> > Kristian
> >
> > 2012/7/26 Igor Fedorenko <ig...@ifedorenko.com>:
> >> I have not looked at parallel execution closely and don't fully
> >> understand the usecase, but I am not sure race condition is the code in
> >> DefaultClassRealmManager.
> >>
> >> As far as I can tell, DefaultClassRealmManager only provides create*ock
> >> methods, which create new realms and the current implementation looks
> >> proper.
> >>
> >> If parallel execution logic needs to be able to create-or-query plugin
> >> realms in thread-safe manner, I think this should be implemented
> >> somewhere at higher levels, where one-plugin-realm-per-plugin-perproject
> >> constraint can be enforced properly.
> >>
> >> Again, I do not know parallel execution, so I can be completely wrong
> >> about this.
> >>
> >> --
> >> Regards,
> >> Igor
> >>
> >> On 12-07-26 12:36 PM, Jason van Zyl wrote:
> >>> In that case we may just want to differentiate because the parallel
> >>> execution use case is not one that happens inside the IDE. In the case
> >>> of
> >>> CLI execution it can all be handled up front as additional realm
> >>> construction beyond the CLI execution isn't going to happen. In an IDE
> >>> or
> >>> shell wouldn't a map of realms keyed by GAV suffice? Sync on write
> >>> everyone
> >>> is free to read?
> >>>
> >>> On Jul 26, 2012, at 12:06 PM, Igor Fedorenko wrote:
> >>>> This is required in m2e where multiple versions of the same plugin can
> >>>> coexist for long time in the same container.
> >>>>
> >>>> --
> >>>> Regards,
> >>>> Igor
> >>>>
> >>>> On 12-07-26 8:44 AM, Kristian Rosenvold wrote:
> >>>>> There is a race condition in parallel builds that occurs related to
> >>>>> this piece of code:
> >>>>>
> >>>>>
> >>>>> http://maven.apache.org/ref/3.0.4/maven-core/xref/org/apache/maven/cla
> >>>>> ssrealm/DefaultClassRealmManager.html#75
> >>>>>
> >>>>> The thing is, for some reason, there's a loop that retries the class
> >>>>> realm generation with a random suffix if the class realm already
> >>>>> exists. In a parallel run, there will be multiple threads requesting
> >>>>> the same realm-id, which semantically should map to the same instance
> >>>>> of the class realm.
> >>>>>
> >>>>> (Most plugins do not really mind if there's a duplicate class realm
> >>>>> every now and then, but some take it very seriously ;)
> >>>>>
> >>>>> I'm tempted to change the semantics of the "newRealm" method to
> >>>>> "getOrCreateRealm", since that seems to be the correct semantics no
> >>>>> matter what. I've tried tracking the origin of the while loop, and it
> >>>>> seems to be very old. Anyone have any idea of what purpose it served ?
> >>>>>
> >>>>> Kristian
> >>>>>
> >>>>> ---------------------------------------------------------------------
> >>>>> 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
> >>>
> >>> Thanks,
> >>>
> >>> Jason
> >>>
> >>> ----------------------------------------------------------
> >>> Jason van Zyl
> >>> Founder & CTO, Sonatype
> >>> Founder, Apache Maven
> >>> http://twitter.com/jvanzyl
> >>> ---------------------------------------------------------
> >>>
> >>> The modern conservative is engaged in one of man's oldest exercises in
> >>> moral philosophy; that is,
> >>> the search for a superior moral justification for selfishness.
> >>>
> >>> -- John Kenneth Galbraith
> >>
> >> ---------------------------------------------------------------------
> >> 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
>
> ---------------------------------------------------------------------
> 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: Use case for funky class realm construction logic
Posted by Igor Fedorenko <ig...@ifedorenko.com>.
On 12-07-27 8:48 AM, Kristian Rosenvold wrote:
> I agree, DefaultClassRealmManager does just what it says; you asked
> for a new instance you'll get one; no need to change that. The answer
> to my original question seems to be that the funky logic is not there
> for maven cli usage, but for IDE integrations. I'm not too happy about
> this code being in core at all, hopefully there's at least a testcase
> that will break if I remove the retry logic ;) But I dont use eclipse
> so I'm leaving that behind.
>
I do not believe this is the case. Parallel execution and m2e are
similar when it comes to class realm management. In both cases it is
likely to have multiple instances of the same plugin coexist, so m2e
should be fine if you get rid of this logic without breaking anything
for parallel execution.
> It looks to me like the test in
> DefaultBuildPluginManager#getPluginRealm is just an optimization that
> allows avoiding the lookup in
> DefaultMavenPluginManager#pluginRealmCache. I could remove the whole
> lookup and I'd just waste a few more cpu cycles building the cache key
> and doing the lookup.
>
> The heart of the problem seems to be some kind of race over the data
> being used to create the cache key, I'm adding some logging of the
> values in question to try to smoke it out. I'm not convinved
> synchronizing getPluginRealm will help in this case.
>
>
getPluginRealm is lazy initializing PluginDescriptor.classRealm using
slightly disguised double-check-locking pattern [1]. I'd still
synchronize it, just to be safe.
I would not remove getPluginRealm, however. setupPluginRealm resets some
parts of PluginDescriptor, ComponentDescriptor.implementationClass in
particular, which will likely cause problems when multiple threads share
the same PluginDescriptor instance.
[1] http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html
--
Regards,
Igor
> Kristian
>
> 2012/7/27 Igor Fedorenko <ig...@ifedorenko.com>:
>> Shouldn't DefaultBuildPluginManager#getPluginRealm be synchronized?
>>
>> If two threads arrive at DefaultBuildPluginManager line 170 at about the
>> same time, they both will see null pluginRealm and both will then call
>> setupPluginRealm, which I believe will likely result in the two threads
>> using two different classloaders.
>>
>> Also, I still do not believe there is anything wrong with
>> design/implementation of DefaultClassRealmManager. If it is asked to
>> createPluginRealm multiple times, it creates multiple realms. Doing
>> otherwise will result in the same realm being reused for dirrefent
>> project/plugin combos and will mess up classloading unless entire
>> classloading approach is reworked.
>>
>> https://cwiki.apache.org/MAVEN/maven-3x-class-loading.html
>>
>> --
>> Regards,
>> Igor
>>
>>
>> On 12-07-26 1:27 PM, Kristian Rosenvold wrote:
>>>
>>> I have some very specific logs in an issue
>>>
>>> (http://code.google.com/p/maven-svn-revision-number-plugin/issues/detail?id=16#c85)
>>> indicating that to class realms are being created for the same plugin
>>> when there is only one execution.
>>>
>>> The behaviour seems totally consistent with hitting the catch block at
>>> line 100 of DefaultClassRealmManager, down to
>>> the suffix of the second class realm. Logically, this will happen at
>>> any time when the FIRST construction of the class realm happens in two
>>> parallel modules in the build. Imagine that you have two modules at
>>> the end of the build, both initializing the war plugin. If they are
>>> siblings in the parallel scheduling, the likelyhood of these two
>>> requesting realm in parallel is extremely high.
>>>
>>> The thread scheduling will ensure that two such modules will arrive at
>>> DefaultBuildPluginManager line 170 at more or less the same instant.
>>> Both will go through the if test, since this is the first time the
>>> realm is created. The interesting thing is that it seems to me like
>>> the cache key at line 304 must somehow be different, since the
>>> setupPluginRealm method is synchronized.
>>>
>>> It seems like I need to follow up on why the cache key is different,
>>> because technically it looks like we should be avoiding the second
>>> call to newRealm, meaning I should really be able to leave the funky
>>> logic as-is.
>>>
>>> WDYT ?
>>>
>>> Kristian
>>>
>>>
>>> 2012/7/26 Igor Fedorenko <ig...@ifedorenko.com>:
>>>>
>>>> I have not looked at parallel execution closely and don't fully
>>>> understand the usecase, but I am not sure race condition is the code in
>>>> DefaultClassRealmManager.
>>>>
>>>> As far as I can tell, DefaultClassRealmManager only provides create*ock
>>>> methods, which create new realms and the current implementation looks
>>>> proper.
>>>>
>>>> If parallel execution logic needs to be able to create-or-query plugin
>>>> realms in thread-safe manner, I think this should be implemented
>>>> somewhere at higher levels, where one-plugin-realm-per-plugin-perproject
>>>> constraint can be enforced properly.
>>>>
>>>> Again, I do not know parallel execution, so I can be completely wrong
>>>> about this.
>>>>
>>>> --
>>>> Regards,
>>>> Igor
>>>>
>>>>
>>>> On 12-07-26 12:36 PM, Jason van Zyl wrote:
>>>>>
>>>>>
>>>>> In that case we may just want to differentiate because the parallel
>>>>> execution use case is not one that happens inside the IDE. In the case
>>>>> of
>>>>> CLI execution it can all be handled up front as additional realm
>>>>> construction beyond the CLI execution isn't going to happen. In an IDE
>>>>> or
>>>>> shell wouldn't a map of realms keyed by GAV suffice? Sync on write
>>>>> everyone
>>>>> is free to read?
>>>>>
>>>>> On Jul 26, 2012, at 12:06 PM, Igor Fedorenko wrote:
>>>>>
>>>>>> This is required in m2e where multiple versions of the same plugin can
>>>>>> coexist for long time in the same container.
>>>>>>
>>>>>> --
>>>>>> Regards,
>>>>>> Igor
>>>>>>
>>>>>> On 12-07-26 8:44 AM, Kristian Rosenvold wrote:
>>>>>>>
>>>>>>>
>>>>>>> There is a race condition in parallel builds that occurs related to
>>>>>>> this piece of code:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> http://maven.apache.org/ref/3.0.4/maven-core/xref/org/apache/maven/classrealm/DefaultClassRealmManager.html#75
>>>>>>>
>>>>>>> The thing is, for some reason, there's a loop that retries the class
>>>>>>> realm generation with a random suffix if the class realm already
>>>>>>> exists. In a parallel run, there will be multiple threads requesting
>>>>>>> the same realm-id, which semantically should map to the same instance
>>>>>>> of the class realm.
>>>>>>>
>>>>>>> (Most plugins do not really mind if there's a duplicate class realm
>>>>>>> every now and then, but some take it very seriously ;)
>>>>>>>
>>>>>>> I'm tempted to change the semantics of the "newRealm" method to
>>>>>>> "getOrCreateRealm", since that seems to be the correct semantics no
>>>>>>> matter what. I've tried tracking the origin of the while loop, and it
>>>>>>> seems to be very old. Anyone have any idea of what purpose it served ?
>>>>>>>
>>>>>>> Kristian
>>>>>>>
>>>>>>> ---------------------------------------------------------------------
>>>>>>> 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
>>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Jason
>>>>>
>>>>> ----------------------------------------------------------
>>>>> Jason van Zyl
>>>>> Founder & CTO, Sonatype
>>>>> Founder, Apache Maven
>>>>> http://twitter.com/jvanzyl
>>>>> ---------------------------------------------------------
>>>>>
>>>>> The modern conservative is engaged in one of man's oldest exercises in
>>>>> moral philosophy; that is,
>>>>> the search for a superior moral justification for selfishness.
>>>>>
>>>>> -- John Kenneth Galbraith
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> 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
>>>
>>
>> ---------------------------------------------------------------------
>> 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
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: Use case for funky class realm construction logic
Posted by Kristian Rosenvold <kr...@gmail.com>.
I agree, DefaultClassRealmManager does just what it says; you asked
for a new instance you'll get one; no need to change that. The answer
to my original question seems to be that the funky logic is not there
for maven cli usage, but for IDE integrations. I'm not too happy about
this code being in core at all, hopefully there's at least a testcase
that will break if I remove the retry logic ;) But I dont use eclipse
so I'm leaving that behind.
It looks to me like the test in
DefaultBuildPluginManager#getPluginRealm is just an optimization that
allows avoiding the lookup in
DefaultMavenPluginManager#pluginRealmCache. I could remove the whole
lookup and I'd just waste a few more cpu cycles building the cache key
and doing the lookup.
The heart of the problem seems to be some kind of race over the data
being used to create the cache key, I'm adding some logging of the
values in question to try to smoke it out. I'm not convinved
synchronizing getPluginRealm will help in this case.
Kristian
2012/7/27 Igor Fedorenko <ig...@ifedorenko.com>:
> Shouldn't DefaultBuildPluginManager#getPluginRealm be synchronized?
>
> If two threads arrive at DefaultBuildPluginManager line 170 at about the
> same time, they both will see null pluginRealm and both will then call
> setupPluginRealm, which I believe will likely result in the two threads
> using two different classloaders.
>
> Also, I still do not believe there is anything wrong with
> design/implementation of DefaultClassRealmManager. If it is asked to
> createPluginRealm multiple times, it creates multiple realms. Doing
> otherwise will result in the same realm being reused for dirrefent
> project/plugin combos and will mess up classloading unless entire
> classloading approach is reworked.
>
> https://cwiki.apache.org/MAVEN/maven-3x-class-loading.html
>
> --
> Regards,
> Igor
>
>
> On 12-07-26 1:27 PM, Kristian Rosenvold wrote:
>>
>> I have some very specific logs in an issue
>>
>> (http://code.google.com/p/maven-svn-revision-number-plugin/issues/detail?id=16#c85)
>> indicating that to class realms are being created for the same plugin
>> when there is only one execution.
>>
>> The behaviour seems totally consistent with hitting the catch block at
>> line 100 of DefaultClassRealmManager, down to
>> the suffix of the second class realm. Logically, this will happen at
>> any time when the FIRST construction of the class realm happens in two
>> parallel modules in the build. Imagine that you have two modules at
>> the end of the build, both initializing the war plugin. If they are
>> siblings in the parallel scheduling, the likelyhood of these two
>> requesting realm in parallel is extremely high.
>>
>> The thread scheduling will ensure that two such modules will arrive at
>> DefaultBuildPluginManager line 170 at more or less the same instant.
>> Both will go through the if test, since this is the first time the
>> realm is created. The interesting thing is that it seems to me like
>> the cache key at line 304 must somehow be different, since the
>> setupPluginRealm method is synchronized.
>>
>> It seems like I need to follow up on why the cache key is different,
>> because technically it looks like we should be avoiding the second
>> call to newRealm, meaning I should really be able to leave the funky
>> logic as-is.
>>
>> WDYT ?
>>
>> Kristian
>>
>>
>> 2012/7/26 Igor Fedorenko <ig...@ifedorenko.com>:
>>>
>>> I have not looked at parallel execution closely and don't fully
>>> understand the usecase, but I am not sure race condition is the code in
>>> DefaultClassRealmManager.
>>>
>>> As far as I can tell, DefaultClassRealmManager only provides create*ock
>>> methods, which create new realms and the current implementation looks
>>> proper.
>>>
>>> If parallel execution logic needs to be able to create-or-query plugin
>>> realms in thread-safe manner, I think this should be implemented
>>> somewhere at higher levels, where one-plugin-realm-per-plugin-perproject
>>> constraint can be enforced properly.
>>>
>>> Again, I do not know parallel execution, so I can be completely wrong
>>> about this.
>>>
>>> --
>>> Regards,
>>> Igor
>>>
>>>
>>> On 12-07-26 12:36 PM, Jason van Zyl wrote:
>>>>
>>>>
>>>> In that case we may just want to differentiate because the parallel
>>>> execution use case is not one that happens inside the IDE. In the case
>>>> of
>>>> CLI execution it can all be handled up front as additional realm
>>>> construction beyond the CLI execution isn't going to happen. In an IDE
>>>> or
>>>> shell wouldn't a map of realms keyed by GAV suffice? Sync on write
>>>> everyone
>>>> is free to read?
>>>>
>>>> On Jul 26, 2012, at 12:06 PM, Igor Fedorenko wrote:
>>>>
>>>>> This is required in m2e where multiple versions of the same plugin can
>>>>> coexist for long time in the same container.
>>>>>
>>>>> --
>>>>> Regards,
>>>>> Igor
>>>>>
>>>>> On 12-07-26 8:44 AM, Kristian Rosenvold wrote:
>>>>>>
>>>>>>
>>>>>> There is a race condition in parallel builds that occurs related to
>>>>>> this piece of code:
>>>>>>
>>>>>>
>>>>>>
>>>>>> http://maven.apache.org/ref/3.0.4/maven-core/xref/org/apache/maven/classrealm/DefaultClassRealmManager.html#75
>>>>>>
>>>>>> The thing is, for some reason, there's a loop that retries the class
>>>>>> realm generation with a random suffix if the class realm already
>>>>>> exists. In a parallel run, there will be multiple threads requesting
>>>>>> the same realm-id, which semantically should map to the same instance
>>>>>> of the class realm.
>>>>>>
>>>>>> (Most plugins do not really mind if there's a duplicate class realm
>>>>>> every now and then, but some take it very seriously ;)
>>>>>>
>>>>>> I'm tempted to change the semantics of the "newRealm" method to
>>>>>> "getOrCreateRealm", since that seems to be the correct semantics no
>>>>>> matter what. I've tried tracking the origin of the while loop, and it
>>>>>> seems to be very old. Anyone have any idea of what purpose it served ?
>>>>>>
>>>>>> Kristian
>>>>>>
>>>>>> ---------------------------------------------------------------------
>>>>>> 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
>>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Jason
>>>>
>>>> ----------------------------------------------------------
>>>> Jason van Zyl
>>>> Founder & CTO, Sonatype
>>>> Founder, Apache Maven
>>>> http://twitter.com/jvanzyl
>>>> ---------------------------------------------------------
>>>>
>>>> The modern conservative is engaged in one of man's oldest exercises in
>>>> moral philosophy; that is,
>>>> the search for a superior moral justification for selfishness.
>>>>
>>>> -- John Kenneth Galbraith
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>> ---------------------------------------------------------------------
>>> 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
>>
>
> ---------------------------------------------------------------------
> 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: Use case for funky class realm construction logic
Posted by Igor Fedorenko <ig...@ifedorenko.com>.
Shouldn't DefaultBuildPluginManager#getPluginRealm be synchronized?
If two threads arrive at DefaultBuildPluginManager line 170 at about the
same time, they both will see null pluginRealm and both will then call
setupPluginRealm, which I believe will likely result in the two threads
using two different classloaders.
Also, I still do not believe there is anything wrong with
design/implementation of DefaultClassRealmManager. If it is asked to
createPluginRealm multiple times, it creates multiple realms. Doing
otherwise will result in the same realm being reused for dirrefent
project/plugin combos and will mess up classloading unless entire
classloading approach is reworked.
https://cwiki.apache.org/MAVEN/maven-3x-class-loading.html
--
Regards,
Igor
On 12-07-26 1:27 PM, Kristian Rosenvold wrote:
> I have some very specific logs in an issue
> (http://code.google.com/p/maven-svn-revision-number-plugin/issues/detail?id=16#c85)
> indicating that to class realms are being created for the same plugin
> when there is only one execution.
>
> The behaviour seems totally consistent with hitting the catch block at
> line 100 of DefaultClassRealmManager, down to
> the suffix of the second class realm. Logically, this will happen at
> any time when the FIRST construction of the class realm happens in two
> parallel modules in the build. Imagine that you have two modules at
> the end of the build, both initializing the war plugin. If they are
> siblings in the parallel scheduling, the likelyhood of these two
> requesting realm in parallel is extremely high.
>
> The thread scheduling will ensure that two such modules will arrive at
> DefaultBuildPluginManager line 170 at more or less the same instant.
> Both will go through the if test, since this is the first time the
> realm is created. The interesting thing is that it seems to me like
> the cache key at line 304 must somehow be different, since the
> setupPluginRealm method is synchronized.
>
> It seems like I need to follow up on why the cache key is different,
> because technically it looks like we should be avoiding the second
> call to newRealm, meaning I should really be able to leave the funky
> logic as-is.
>
> WDYT ?
>
> Kristian
>
>
> 2012/7/26 Igor Fedorenko <ig...@ifedorenko.com>:
>> I have not looked at parallel execution closely and don't fully
>> understand the usecase, but I am not sure race condition is the code in
>> DefaultClassRealmManager.
>>
>> As far as I can tell, DefaultClassRealmManager only provides create*ock
>> methods, which create new realms and the current implementation looks
>> proper.
>>
>> If parallel execution logic needs to be able to create-or-query plugin
>> realms in thread-safe manner, I think this should be implemented
>> somewhere at higher levels, where one-plugin-realm-per-plugin-perproject
>> constraint can be enforced properly.
>>
>> Again, I do not know parallel execution, so I can be completely wrong
>> about this.
>>
>> --
>> Regards,
>> Igor
>>
>>
>> On 12-07-26 12:36 PM, Jason van Zyl wrote:
>>>
>>> In that case we may just want to differentiate because the parallel
>>> execution use case is not one that happens inside the IDE. In the case of
>>> CLI execution it can all be handled up front as additional realm
>>> construction beyond the CLI execution isn't going to happen. In an IDE or
>>> shell wouldn't a map of realms keyed by GAV suffice? Sync on write everyone
>>> is free to read?
>>>
>>> On Jul 26, 2012, at 12:06 PM, Igor Fedorenko wrote:
>>>
>>>> This is required in m2e where multiple versions of the same plugin can
>>>> coexist for long time in the same container.
>>>>
>>>> --
>>>> Regards,
>>>> Igor
>>>>
>>>> On 12-07-26 8:44 AM, Kristian Rosenvold wrote:
>>>>>
>>>>> There is a race condition in parallel builds that occurs related to
>>>>> this piece of code:
>>>>>
>>>>>
>>>>> http://maven.apache.org/ref/3.0.4/maven-core/xref/org/apache/maven/classrealm/DefaultClassRealmManager.html#75
>>>>>
>>>>> The thing is, for some reason, there's a loop that retries the class
>>>>> realm generation with a random suffix if the class realm already
>>>>> exists. In a parallel run, there will be multiple threads requesting
>>>>> the same realm-id, which semantically should map to the same instance
>>>>> of the class realm.
>>>>>
>>>>> (Most plugins do not really mind if there's a duplicate class realm
>>>>> every now and then, but some take it very seriously ;)
>>>>>
>>>>> I'm tempted to change the semantics of the "newRealm" method to
>>>>> "getOrCreateRealm", since that seems to be the correct semantics no
>>>>> matter what. I've tried tracking the origin of the while loop, and it
>>>>> seems to be very old. Anyone have any idea of what purpose it served ?
>>>>>
>>>>> Kristian
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> 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
>>>>
>>>
>>> Thanks,
>>>
>>> Jason
>>>
>>> ----------------------------------------------------------
>>> Jason van Zyl
>>> Founder & CTO, Sonatype
>>> Founder, Apache Maven
>>> http://twitter.com/jvanzyl
>>> ---------------------------------------------------------
>>>
>>> The modern conservative is engaged in one of man's oldest exercises in
>>> moral philosophy; that is,
>>> the search for a superior moral justification for selfishness.
>>>
>>> -- John Kenneth Galbraith
>>>
>>>
>>>
>>>
>>>
>>>
>>
>> ---------------------------------------------------------------------
>> 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
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: Use case for funky class realm construction logic
Posted by Kristian Rosenvold <kr...@gmail.com>.
I have some very specific logs in an issue
(http://code.google.com/p/maven-svn-revision-number-plugin/issues/detail?id=16#c85)
indicating that to class realms are being created for the same plugin
when there is only one execution.
The behaviour seems totally consistent with hitting the catch block at
line 100 of DefaultClassRealmManager, down to
the suffix of the second class realm. Logically, this will happen at
any time when the FIRST construction of the class realm happens in two
parallel modules in the build. Imagine that you have two modules at
the end of the build, both initializing the war plugin. If they are
siblings in the parallel scheduling, the likelyhood of these two
requesting realm in parallel is extremely high.
The thread scheduling will ensure that two such modules will arrive at
DefaultBuildPluginManager line 170 at more or less the same instant.
Both will go through the if test, since this is the first time the
realm is created. The interesting thing is that it seems to me like
the cache key at line 304 must somehow be different, since the
setupPluginRealm method is synchronized.
It seems like I need to follow up on why the cache key is different,
because technically it looks like we should be avoiding the second
call to newRealm, meaning I should really be able to leave the funky
logic as-is.
WDYT ?
Kristian
2012/7/26 Igor Fedorenko <ig...@ifedorenko.com>:
> I have not looked at parallel execution closely and don't fully
> understand the usecase, but I am not sure race condition is the code in
> DefaultClassRealmManager.
>
> As far as I can tell, DefaultClassRealmManager only provides create*ock
> methods, which create new realms and the current implementation looks
> proper.
>
> If parallel execution logic needs to be able to create-or-query plugin
> realms in thread-safe manner, I think this should be implemented
> somewhere at higher levels, where one-plugin-realm-per-plugin-perproject
> constraint can be enforced properly.
>
> Again, I do not know parallel execution, so I can be completely wrong
> about this.
>
> --
> Regards,
> Igor
>
>
> On 12-07-26 12:36 PM, Jason van Zyl wrote:
>>
>> In that case we may just want to differentiate because the parallel
>> execution use case is not one that happens inside the IDE. In the case of
>> CLI execution it can all be handled up front as additional realm
>> construction beyond the CLI execution isn't going to happen. In an IDE or
>> shell wouldn't a map of realms keyed by GAV suffice? Sync on write everyone
>> is free to read?
>>
>> On Jul 26, 2012, at 12:06 PM, Igor Fedorenko wrote:
>>
>>> This is required in m2e where multiple versions of the same plugin can
>>> coexist for long time in the same container.
>>>
>>> --
>>> Regards,
>>> Igor
>>>
>>> On 12-07-26 8:44 AM, Kristian Rosenvold wrote:
>>>>
>>>> There is a race condition in parallel builds that occurs related to
>>>> this piece of code:
>>>>
>>>>
>>>> http://maven.apache.org/ref/3.0.4/maven-core/xref/org/apache/maven/classrealm/DefaultClassRealmManager.html#75
>>>>
>>>> The thing is, for some reason, there's a loop that retries the class
>>>> realm generation with a random suffix if the class realm already
>>>> exists. In a parallel run, there will be multiple threads requesting
>>>> the same realm-id, which semantically should map to the same instance
>>>> of the class realm.
>>>>
>>>> (Most plugins do not really mind if there's a duplicate class realm
>>>> every now and then, but some take it very seriously ;)
>>>>
>>>> I'm tempted to change the semantics of the "newRealm" method to
>>>> "getOrCreateRealm", since that seems to be the correct semantics no
>>>> matter what. I've tried tracking the origin of the while loop, and it
>>>> seems to be very old. Anyone have any idea of what purpose it served ?
>>>>
>>>> Kristian
>>>>
>>>> ---------------------------------------------------------------------
>>>> 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
>>>
>>
>> Thanks,
>>
>> Jason
>>
>> ----------------------------------------------------------
>> Jason van Zyl
>> Founder & CTO, Sonatype
>> Founder, Apache Maven
>> http://twitter.com/jvanzyl
>> ---------------------------------------------------------
>>
>> The modern conservative is engaged in one of man's oldest exercises in
>> moral philosophy; that is,
>> the search for a superior moral justification for selfishness.
>>
>> -- John Kenneth Galbraith
>>
>>
>>
>>
>>
>>
>
> ---------------------------------------------------------------------
> 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: Use case for funky class realm construction logic
Posted by Igor Fedorenko <ig...@ifedorenko.com>.
I have not looked at parallel execution closely and don't fully
understand the usecase, but I am not sure race condition is the code in
DefaultClassRealmManager.
As far as I can tell, DefaultClassRealmManager only provides create*
methods, which create new realms and the current implementation looks
proper.
If parallel execution logic needs to be able to create-or-query plugin
realms in thread-safe manner, I think this should be implemented
somewhere at higher levels, where one-plugin-realm-per-plugin-perproject
constraint can be enforced properly.
Again, I do not know parallel execution, so I can be completely wrong
about this.
--
Regards,
Igor
On 12-07-26 12:36 PM, Jason van Zyl wrote:
> In that case we may just want to differentiate because the parallel execution use case is not one that happens inside the IDE. In the case of CLI execution it can all be handled up front as additional realm construction beyond the CLI execution isn't going to happen. In an IDE or shell wouldn't a map of realms keyed by GAV suffice? Sync on write everyone is free to read?
>
> On Jul 26, 2012, at 12:06 PM, Igor Fedorenko wrote:
>
>> This is required in m2e where multiple versions of the same plugin can
>> coexist for long time in the same container.
>>
>> --
>> Regards,
>> Igor
>>
>> On 12-07-26 8:44 AM, Kristian Rosenvold wrote:
>>> There is a race condition in parallel builds that occurs related to
>>> this piece of code:
>>>
>>> http://maven.apache.org/ref/3.0.4/maven-core/xref/org/apache/maven/classrealm/DefaultClassRealmManager.html#75
>>>
>>> The thing is, for some reason, there's a loop that retries the class
>>> realm generation with a random suffix if the class realm already
>>> exists. In a parallel run, there will be multiple threads requesting
>>> the same realm-id, which semantically should map to the same instance
>>> of the class realm.
>>>
>>> (Most plugins do not really mind if there's a duplicate class realm
>>> every now and then, but some take it very seriously ;)
>>>
>>> I'm tempted to change the semantics of the "newRealm" method to
>>> "getOrCreateRealm", since that seems to be the correct semantics no
>>> matter what. I've tried tracking the origin of the while loop, and it
>>> seems to be very old. Anyone have any idea of what purpose it served ?
>>>
>>> Kristian
>>>
>>> ---------------------------------------------------------------------
>>> 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
>>
>
> Thanks,
>
> Jason
>
> ----------------------------------------------------------
> Jason van Zyl
> Founder & CTO, Sonatype
> Founder, Apache Maven
> http://twitter.com/jvanzyl
> ---------------------------------------------------------
>
> The modern conservative is engaged in one of man's oldest exercises in moral philosophy; that is,
> the search for a superior moral justification for selfishness.
>
> -- John Kenneth Galbraith
>
>
>
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: Use case for funky class realm construction logic
Posted by Jason van Zyl <ja...@tesla.io>.
In that case we may just want to differentiate because the parallel execution use case is not one that happens inside the IDE. In the case of CLI execution it can all be handled up front as additional realm construction beyond the CLI execution isn't going to happen. In an IDE or shell wouldn't a map of realms keyed by GAV suffice? Sync on write everyone is free to read?
On Jul 26, 2012, at 12:06 PM, Igor Fedorenko wrote:
> This is required in m2e where multiple versions of the same plugin can
> coexist for long time in the same container.
>
> --
> Regards,
> Igor
>
> On 12-07-26 8:44 AM, Kristian Rosenvold wrote:
>> There is a race condition in parallel builds that occurs related to
>> this piece of code:
>>
>> http://maven.apache.org/ref/3.0.4/maven-core/xref/org/apache/maven/classrealm/DefaultClassRealmManager.html#75
>>
>> The thing is, for some reason, there's a loop that retries the class
>> realm generation with a random suffix if the class realm already
>> exists. In a parallel run, there will be multiple threads requesting
>> the same realm-id, which semantically should map to the same instance
>> of the class realm.
>>
>> (Most plugins do not really mind if there's a duplicate class realm
>> every now and then, but some take it very seriously ;)
>>
>> I'm tempted to change the semantics of the "newRealm" method to
>> "getOrCreateRealm", since that seems to be the correct semantics no
>> matter what. I've tried tracking the origin of the while loop, and it
>> seems to be very old. Anyone have any idea of what purpose it served ?
>>
>> Kristian
>>
>> ---------------------------------------------------------------------
>> 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
>
Thanks,
Jason
----------------------------------------------------------
Jason van Zyl
Founder & CTO, Sonatype
Founder, Apache Maven
http://twitter.com/jvanzyl
---------------------------------------------------------
The modern conservative is engaged in one of man's oldest exercises in moral philosophy; that is,
the search for a superior moral justification for selfishness.
-- John Kenneth Galbraith
Re: Use case for funky class realm construction logic
Posted by Igor Fedorenko <ig...@ifedorenko.com>.
This is required in m2e where multiple versions of the same plugin can
coexist for long time in the same container.
--
Regards,
Igor
On 12-07-26 8:44 AM, Kristian Rosenvold wrote:
> There is a race condition in parallel builds that occurs related to
> this piece of code:
>
> http://maven.apache.org/ref/3.0.4/maven-core/xref/org/apache/maven/classrealm/DefaultClassRealmManager.html#75
>
> The thing is, for some reason, there's a loop that retries the class
> realm generation with a random suffix if the class realm already
> exists. In a parallel run, there will be multiple threads requesting
> the same realm-id, which semantically should map to the same instance
> of the class realm.
>
> (Most plugins do not really mind if there's a duplicate class realm
> every now and then, but some take it very seriously ;)
>
> I'm tempted to change the semantics of the "newRealm" method to
> "getOrCreateRealm", since that seems to be the correct semantics no
> matter what. I've tried tracking the origin of the while loop, and it
> seems to be very old. Anyone have any idea of what purpose it served ?
>
> Kristian
>
> ---------------------------------------------------------------------
> 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: Use case for funky class realm construction logic
Posted by Jason van Zyl <ja...@tesla.io>.
I'm just surfing around some code now, but I think it may be for a long-lived process like an IDE some of that weirdness exists.
On Jul 26, 2012, at 8:44 AM, Kristian Rosenvold wrote:
> There is a race condition in parallel builds that occurs related to
> this piece of code:
>
> http://maven.apache.org/ref/3.0.4/maven-core/xref/org/apache/maven/classrealm/DefaultClassRealmManager.html#75
>
> The thing is, for some reason, there's a loop that retries the class
> realm generation with a random suffix if the class realm already
> exists. In a parallel run, there will be multiple threads requesting
> the same realm-id, which semantically should map to the same instance
> of the class realm.
>
> (Most plugins do not really mind if there's a duplicate class realm
> every now and then, but some take it very seriously ;)
>
> I'm tempted to change the semantics of the "newRealm" method to
> "getOrCreateRealm", since that seems to be the correct semantics no
> matter what. I've tried tracking the origin of the while loop, and it
> seems to be very old. Anyone have any idea of what purpose it served ?
>
> Kristian
>
> ---------------------------------------------------------------------
> 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 & CTO, Sonatype
Founder, Apache Maven
http://twitter.com/jvanzyl
---------------------------------------------------------
I never make the mistake of arguing with people for whose opinions I have no respect.
-- Edward Gibbon
Re: Use case for funky class realm construction logic
Posted by Jason van Zyl <ja...@tesla.io>.
To solve your problem, it may be that code is truly only useful to m2e and if so we should probably find a way to allow m2e to do what's necessary but not interfere with the CLI use case. Maybe a simple protected method where m2e can have an implementation that overrides the realm creation bits would probably suffice.
On Jul 26, 2012, at 7:35 PM, Kristian Rosenvold wrote:
> I think you're right about the 95% overlap. If we download the whole
> plugin with all their deps to calculate the execution plan we could
> surely construct the class realm too; that's just a few pojos per
> plugin. I'll investigate.
>
> Still this whole discussion is probably tangential to the problem I
> set out to resolve, which seems to involve a race condition around the
> cache key.
>
> Kristian
>
>
> 2012/7/27 Jason van Zyl <ja...@tesla.io>:
>> I wouldn't speculate without measuring but from anecdotal experience I would venture to say that of those 200 modules there's 95% overlap in the plugins executed. I'm not sure it would be that inefficient given what we do today which is to download the plugin itself to get at its metadata to build the execution plan. But I also can't see why there would be any contention in a short-lived process. I'm also not terribly familiar with the parallel execution code (I looked at once when you first did it and it just worked at that point so I didn't look much again) so I'll take a peek this weekend.
>>
>> On Jul 26, 2012, at 2:10 PM, Kristian Rosenvold wrote:
>>
>>> I'm not sure this makes sense. Given a reactor with 200 modules, it
>>> would mean you would be downloading all the plugins and constructing
>>> all the realms before even starting on the first module ?
>>>
>>> Re-using realms across modules is great, but pre-constructing
>>> everything would give an uneccessary performance hit ?
>>>
>>> Kristian
>>>
>>> 2012/7/26 Jason van Zyl <ja...@tesla.io>:
>>>> Could we not adjust such that after the execution plan is created, prepare all the class realms required by the plan and once constructed left immutable during the actual execution of the plan. I can't quite remember but I'm not sure why we need multiple threads constructing realms with the execution plan in hand. Prepare the realms according to the plan, make them immutable and use them throughout the session.
>>>>
>>>> On Jul 26, 2012, at 8:44 AM, Kristian Rosenvold wrote:
>>>>
>>>>> There is a race condition in parallel builds that occurs related to
>>>>> this piece of code:
>>>>>
>>>>> http://maven.apache.org/ref/3.0.4/maven-core/xref/org/apache/maven/classrealm/DefaultClassRealmManager.html#75
>>>>>
>>>>> The thing is, for some reason, there's a loop that retries the class
>>>>> realm generation with a random suffix if the class realm already
>>>>> exists. In a parallel run, there will be multiple threads requesting
>>>>> the same realm-id, which semantically should map to the same instance
>>>>> of the class realm.
>>>>>
>>>>> (Most plugins do not really mind if there's a duplicate class realm
>>>>> every now and then, but some take it very seriously ;)
>>>>>
>>>>> I'm tempted to change the semantics of the "newRealm" method to
>>>>> "getOrCreateRealm", since that seems to be the correct semantics no
>>>>> matter what. I've tried tracking the origin of the while loop, and it
>>>>> seems to be very old. Anyone have any idea of what purpose it served ?
>>>>>
>>>>> Kristian
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> 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 & CTO, Sonatype
>>>> Founder, Apache Maven
>>>> http://twitter.com/jvanzyl
>>>> ---------------------------------------------------------
>>>>
>>>> Simplex sigillum veri. (Simplicity is the seal of truth.)
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>> ---------------------------------------------------------------------
>>> 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 & CTO, Sonatype
>> Founder, Apache Maven
>> http://twitter.com/jvanzyl
>> ---------------------------------------------------------
>>
>> believe nothing, no matter where you read it,
>> or who has said it,
>> not even if i have said it,
>> unless it agrees with your own reason
>> and your own common sense.
>>
>> -- Buddha
>>
>>
>>
>>
>>
>
> ---------------------------------------------------------------------
> 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 & CTO, Sonatype
Founder, Apache Maven
http://twitter.com/jvanzyl
---------------------------------------------------------
We all have problems. How we deal with them is a measure of our worth.
-- Unknown
Re: Use case for funky class realm construction logic
Posted by Kristian Rosenvold <kr...@gmail.com>.
I think you're right about the 95% overlap. If we download the whole
plugin with all their deps to calculate the execution plan we could
surely construct the class realm too; that's just a few pojos per
plugin. I'll investigate.
Still this whole discussion is probably tangential to the problem I
set out to resolve, which seems to involve a race condition around the
cache key.
Kristian
2012/7/27 Jason van Zyl <ja...@tesla.io>:
> I wouldn't speculate without measuring but from anecdotal experience I would venture to say that of those 200 modules there's 95% overlap in the plugins executed. I'm not sure it would be that inefficient given what we do today which is to download the plugin itself to get at its metadata to build the execution plan. But I also can't see why there would be any contention in a short-lived process. I'm also not terribly familiar with the parallel execution code (I looked at once when you first did it and it just worked at that point so I didn't look much again) so I'll take a peek this weekend.
>
> On Jul 26, 2012, at 2:10 PM, Kristian Rosenvold wrote:
>
>> I'm not sure this makes sense. Given a reactor with 200 modules, it
>> would mean you would be downloading all the plugins and constructing
>> all the realms before even starting on the first module ?
>>
>> Re-using realms across modules is great, but pre-constructing
>> everything would give an uneccessary performance hit ?
>>
>> Kristian
>>
>> 2012/7/26 Jason van Zyl <ja...@tesla.io>:
>>> Could we not adjust such that after the execution plan is created, prepare all the class realms required by the plan and once constructed left immutable during the actual execution of the plan. I can't quite remember but I'm not sure why we need multiple threads constructing realms with the execution plan in hand. Prepare the realms according to the plan, make them immutable and use them throughout the session.
>>>
>>> On Jul 26, 2012, at 8:44 AM, Kristian Rosenvold wrote:
>>>
>>>> There is a race condition in parallel builds that occurs related to
>>>> this piece of code:
>>>>
>>>> http://maven.apache.org/ref/3.0.4/maven-core/xref/org/apache/maven/classrealm/DefaultClassRealmManager.html#75
>>>>
>>>> The thing is, for some reason, there's a loop that retries the class
>>>> realm generation with a random suffix if the class realm already
>>>> exists. In a parallel run, there will be multiple threads requesting
>>>> the same realm-id, which semantically should map to the same instance
>>>> of the class realm.
>>>>
>>>> (Most plugins do not really mind if there's a duplicate class realm
>>>> every now and then, but some take it very seriously ;)
>>>>
>>>> I'm tempted to change the semantics of the "newRealm" method to
>>>> "getOrCreateRealm", since that seems to be the correct semantics no
>>>> matter what. I've tried tracking the origin of the while loop, and it
>>>> seems to be very old. Anyone have any idea of what purpose it served ?
>>>>
>>>> Kristian
>>>>
>>>> ---------------------------------------------------------------------
>>>> 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 & CTO, Sonatype
>>> Founder, Apache Maven
>>> http://twitter.com/jvanzyl
>>> ---------------------------------------------------------
>>>
>>> Simplex sigillum veri. (Simplicity is the seal of truth.)
>>>
>>>
>>>
>>>
>>>
>>
>> ---------------------------------------------------------------------
>> 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 & CTO, Sonatype
> Founder, Apache Maven
> http://twitter.com/jvanzyl
> ---------------------------------------------------------
>
> believe nothing, no matter where you read it,
> or who has said it,
> not even if i have said it,
> unless it agrees with your own reason
> and your own common sense.
>
> -- Buddha
>
>
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: Use case for funky class realm construction logic
Posted by Jason van Zyl <ja...@tesla.io>.
I wouldn't speculate without measuring but from anecdotal experience I would venture to say that of those 200 modules there's 95% overlap in the plugins executed. I'm not sure it would be that inefficient given what we do today which is to download the plugin itself to get at its metadata to build the execution plan. But I also can't see why there would be any contention in a short-lived process. I'm also not terribly familiar with the parallel execution code (I looked at once when you first did it and it just worked at that point so I didn't look much again) so I'll take a peek this weekend.
On Jul 26, 2012, at 2:10 PM, Kristian Rosenvold wrote:
> I'm not sure this makes sense. Given a reactor with 200 modules, it
> would mean you would be downloading all the plugins and constructing
> all the realms before even starting on the first module ?
>
> Re-using realms across modules is great, but pre-constructing
> everything would give an uneccessary performance hit ?
>
> Kristian
>
> 2012/7/26 Jason van Zyl <ja...@tesla.io>:
>> Could we not adjust such that after the execution plan is created, prepare all the class realms required by the plan and once constructed left immutable during the actual execution of the plan. I can't quite remember but I'm not sure why we need multiple threads constructing realms with the execution plan in hand. Prepare the realms according to the plan, make them immutable and use them throughout the session.
>>
>> On Jul 26, 2012, at 8:44 AM, Kristian Rosenvold wrote:
>>
>>> There is a race condition in parallel builds that occurs related to
>>> this piece of code:
>>>
>>> http://maven.apache.org/ref/3.0.4/maven-core/xref/org/apache/maven/classrealm/DefaultClassRealmManager.html#75
>>>
>>> The thing is, for some reason, there's a loop that retries the class
>>> realm generation with a random suffix if the class realm already
>>> exists. In a parallel run, there will be multiple threads requesting
>>> the same realm-id, which semantically should map to the same instance
>>> of the class realm.
>>>
>>> (Most plugins do not really mind if there's a duplicate class realm
>>> every now and then, but some take it very seriously ;)
>>>
>>> I'm tempted to change the semantics of the "newRealm" method to
>>> "getOrCreateRealm", since that seems to be the correct semantics no
>>> matter what. I've tried tracking the origin of the while loop, and it
>>> seems to be very old. Anyone have any idea of what purpose it served ?
>>>
>>> Kristian
>>>
>>> ---------------------------------------------------------------------
>>> 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 & CTO, Sonatype
>> Founder, Apache Maven
>> http://twitter.com/jvanzyl
>> ---------------------------------------------------------
>>
>> Simplex sigillum veri. (Simplicity is the seal of truth.)
>>
>>
>>
>>
>>
>
> ---------------------------------------------------------------------
> 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 & CTO, Sonatype
Founder, Apache Maven
http://twitter.com/jvanzyl
---------------------------------------------------------
believe nothing, no matter where you read it,
or who has said it,
not even if i have said it,
unless it agrees with your own reason
and your own common sense.
-- Buddha
Re: Use case for funky class realm construction logic
Posted by Kristian Rosenvold <kr...@gmail.com>.
I'm not sure this makes sense. Given a reactor with 200 modules, it
would mean you would be downloading all the plugins and constructing
all the realms before even starting on the first module ?
Re-using realms across modules is great, but pre-constructing
everything would give an uneccessary performance hit ?
Kristian
2012/7/26 Jason van Zyl <ja...@tesla.io>:
> Could we not adjust such that after the execution plan is created, prepare all the class realms required by the plan and once constructed left immutable during the actual execution of the plan. I can't quite remember but I'm not sure why we need multiple threads constructing realms with the execution plan in hand. Prepare the realms according to the plan, make them immutable and use them throughout the session.
>
> On Jul 26, 2012, at 8:44 AM, Kristian Rosenvold wrote:
>
>> There is a race condition in parallel builds that occurs related to
>> this piece of code:
>>
>> http://maven.apache.org/ref/3.0.4/maven-core/xref/org/apache/maven/classrealm/DefaultClassRealmManager.html#75
>>
>> The thing is, for some reason, there's a loop that retries the class
>> realm generation with a random suffix if the class realm already
>> exists. In a parallel run, there will be multiple threads requesting
>> the same realm-id, which semantically should map to the same instance
>> of the class realm.
>>
>> (Most plugins do not really mind if there's a duplicate class realm
>> every now and then, but some take it very seriously ;)
>>
>> I'm tempted to change the semantics of the "newRealm" method to
>> "getOrCreateRealm", since that seems to be the correct semantics no
>> matter what. I've tried tracking the origin of the while loop, and it
>> seems to be very old. Anyone have any idea of what purpose it served ?
>>
>> Kristian
>>
>> ---------------------------------------------------------------------
>> 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 & CTO, Sonatype
> Founder, Apache Maven
> http://twitter.com/jvanzyl
> ---------------------------------------------------------
>
> Simplex sigillum veri. (Simplicity is the seal of truth.)
>
>
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: Use case for funky class realm construction logic
Posted by Jason van Zyl <ja...@tesla.io>.
Could we not adjust such that after the execution plan is created, prepare all the class realms required by the plan and once constructed left immutable during the actual execution of the plan. I can't quite remember but I'm not sure why we need multiple threads constructing realms with the execution plan in hand. Prepare the realms according to the plan, make them immutable and use them throughout the session.
On Jul 26, 2012, at 8:44 AM, Kristian Rosenvold wrote:
> There is a race condition in parallel builds that occurs related to
> this piece of code:
>
> http://maven.apache.org/ref/3.0.4/maven-core/xref/org/apache/maven/classrealm/DefaultClassRealmManager.html#75
>
> The thing is, for some reason, there's a loop that retries the class
> realm generation with a random suffix if the class realm already
> exists. In a parallel run, there will be multiple threads requesting
> the same realm-id, which semantically should map to the same instance
> of the class realm.
>
> (Most plugins do not really mind if there's a duplicate class realm
> every now and then, but some take it very seriously ;)
>
> I'm tempted to change the semantics of the "newRealm" method to
> "getOrCreateRealm", since that seems to be the correct semantics no
> matter what. I've tried tracking the origin of the while loop, and it
> seems to be very old. Anyone have any idea of what purpose it served ?
>
> Kristian
>
> ---------------------------------------------------------------------
> 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 & CTO, Sonatype
Founder, Apache Maven
http://twitter.com/jvanzyl
---------------------------------------------------------
Simplex sigillum veri. (Simplicity is the seal of truth.)