First step towards making kernel more classloader-agnostic

[David Jencks <da...@yahoo.com>]

After more work than I expected I have the server running with  
Configuration being basically a pojo rather than constructing the  
classloaders itself in its constructor.  I commited my current work in  
sandbox/djencks/framework together with a patch (cl-factore.patch.1   
-- I seem to have misspelled factory) for the rest of geronimo.  The  
server starts and seems to work although a bunch of testsuite tests  
fail when run automatically.... when I try the same thing by hand they  
seem to work.

Some of the changes:

-- separate classloader construction into a factory
-- track resolved configuration dependencies in a separate object
-- configuration now is a data container

-- I basically eliminated the EditableKernelConfigurationManager which  
is used in a few places to modify existing configurations.  I've  
always thought this ability was a rather bad idea.  I would rather  
replace it with something like always storing the plan into plugins  
(done now with car-maven-plugin, we just need to add it for stuff  
deployed into geronimo directly) and providing a way to edit the plan  
and redeploy the plugin with a new version number (or only letting you  
edit snapshot plugins)

-- There's been a long-standing problem that many of the "auxillary"  
builders can't figure out whether they need to add dependencies to the  
environment until after they get to scan for annotations at which time  
it was too late to modify the classloader.  As a result most of these  
builders always add their dependencies whether or not they are  
actually needed.  I think that since the classloader construction is  
separated from the configuration initialization we can now solve this  
problem and create new classloaders each time we update the  
dependencies.  I haven't verified this but am rather hopeful.

I think that this is a bit of a conceptual simplification of some of  
the work the configuration manager is doing and that it would be ok to  
commit these changes to trunk.  The main objection I could see would  
be to the EditiableConfigurationManager change.  This functionality  
can probably be added back without too much difficulty although I  
really think it is a mistake.

Thoughts?

The main change is in rev 755494, I removed an accidental file in rev  
755496.

I haven't had a chance to look at Gianny's classloader-per-jar patch  
but I'm hopeful that can be merged into this without excessive  
difficulty.

many thanks
david jencks

Re: First step towards making kernel more classloader-agnostic

[Gianny Damour <gi...@optusnet.com.au>]

Hi,

Those are great changes to merge. Well, the xbean-naming comment  
could be improved thought :)

Thanks,
Gianny

On 18/03/2009, at 9:47 PM, Gianny Damour wrote:

> On 18/03/2009, at 6:24 PM, David Jencks wrote:
>
>> After more work than I expected I have the server running with  
>> Configuration being basically a pojo rather than constructing the  
>> classloaders itself in its constructor.  I commited my current  
>> work in sandbox/djencks/framework together with a patch (cl- 
>> factore.patch.1  -- I seem to have misspelled factory) for the  
>> rest of geronimo.  The server starts and seems to work although a  
>> bunch of testsuite tests fail when run automatically.... when I  
>> try the same thing by hand they seem to work.
>>
>> Some of the changes:
>>
>> -- separate classloader construction into a factory
>> -- track resolved configuration dependencies in a separate object
>> -- configuration now is a data container
>>
>> -- I basically eliminated the EditableKernelConfigurationManager  
>> which is used in a few places to modify existing configurations.   
>> I've always thought this ability was a rather bad idea.  I would  
>> rather replace it with something like always storing the plan into  
>> plugins (done now with car-maven-plugin, we just need to add it  
>> for stuff deployed into geronimo directly) and providing a way to  
>> edit the plan and redeploy the plugin with a new version number  
>> (or only letting you edit snapshot plugins)
>>
>> -- There's been a long-standing problem that many of the  
>> "auxillary" builders can't figure out whether they need to add  
>> dependencies to the environment until after they get to scan for  
>> annotations at which time it was too late to modify the  
>> classloader.  As a result most of these builders always add their  
>> dependencies whether or not they are actually needed.  I think  
>> that since the classloader construction is separated from the  
>> configuration initialization we can now solve this problem and  
>> create new classloaders each time we update the dependencies.  I  
>> haven't verified this but am rather hopeful.
>>
>> I think that this is a bit of a conceptual simplification of some  
>> of the work the configuration manager is doing and that it would  
>> be ok to commit these changes to trunk.  The main objection I  
>> could see would be to the EditiableConfigurationManager change.   
>> This functionality can probably be added back without too much  
>> difficulty although I really think it is a mistake.
>>
>> Thoughts?
>>
>> The main change is in rev 755494, I removed an accidental file in  
>> rev 755496.
>>
>> I haven't had a chance to look at Gianny's classloader-per-jar  
>> patch but I'm hopeful that can be merged into this without  
>> excessive difficulty.
>
> Based on a cursory review of the changes, a simple change to  
> JarFileClassLoaderFactory should be enough to merge the proposed  
> changes. If the patch makes sense, then I can easily merge against  
> this branch.
>
> I will review in more details the overall change, however the  
> extraction of a classloader factory is certainly good.
>
> Thanks,
> Gianny
>
>>
>> many thanks
>> david jencks
>>
>

Re: First step towards making kernel more classloader-agnostic

[Gianny Damour <gi...@optusnet.com.au>]

On 18/03/2009, at 6:24 PM, David Jencks wrote:

> After more work than I expected I have the server running with  
> Configuration being basically a pojo rather than constructing the  
> classloaders itself in its constructor.  I commited my current work  
> in sandbox/djencks/framework together with a patch (cl- 
> factore.patch.1  -- I seem to have misspelled factory) for the rest  
> of geronimo.  The server starts and seems to work although a bunch  
> of testsuite tests fail when run automatically.... when I try the  
> same thing by hand they seem to work.
>
> Some of the changes:
>
> -- separate classloader construction into a factory
> -- track resolved configuration dependencies in a separate object
> -- configuration now is a data container
>
> -- I basically eliminated the EditableKernelConfigurationManager  
> which is used in a few places to modify existing configurations.   
> I've always thought this ability was a rather bad idea.  I would  
> rather replace it with something like always storing the plan into  
> plugins (done now with car-maven-plugin, we just need to add it for  
> stuff deployed into geronimo directly) and providing a way to edit  
> the plan and redeploy the plugin with a new version number (or only  
> letting you edit snapshot plugins)
>
> -- There's been a long-standing problem that many of the  
> "auxillary" builders can't figure out whether they need to add  
> dependencies to the environment until after they get to scan for  
> annotations at which time it was too late to modify the  
> classloader.  As a result most of these builders always add their  
> dependencies whether or not they are actually needed.  I think that  
> since the classloader construction is separated from the  
> configuration initialization we can now solve this problem and  
> create new classloaders each time we update the dependencies.  I  
> haven't verified this but am rather hopeful.
>
> I think that this is a bit of a conceptual simplification of some  
> of the work the configuration manager is doing and that it would be  
> ok to commit these changes to trunk.  The main objection I could  
> see would be to the EditiableConfigurationManager change.  This  
> functionality can probably be added back without too much  
> difficulty although I really think it is a mistake.
>
> Thoughts?
>
> The main change is in rev 755494, I removed an accidental file in  
> rev 755496.
>
> I haven't had a chance to look at Gianny's classloader-per-jar  
> patch but I'm hopeful that can be merged into this without  
> excessive difficulty.

Based on a cursory review of the changes, a simple change to  
JarFileClassLoaderFactory should be enough to merge the proposed  
changes. If the patch makes sense, then I can easily merge against  
this branch.

I will review in more details the overall change, however the  
extraction of a classloader factory is certainly good.

Thanks,
Gianny

>
> many thanks
> david jencks
>