Re: [cp-patches] Re: RFC: Class Loader patch to record classwithinitiating class loader

2005-07-28 Thread Archie Cobbs

Robert Lougher wrote:

In my opinion, the logic for deciding whether to load via the bootstrap
or via a classloader, whether to load an array or a "normal" class, 
initialization, and the managing of the cache is best done in the runtime

for performance reasons.


Most of what Robert said above is pretty much true in the same way
for JCVM, so I agree :-)

Perhaps there is some consensus to make this API simpler instead
of more complicated.

If we decide to "keep it simple", what would definitely be helpful
is comprehensive comments for these native methods sketching out what
the VM should be doing (e.g., it's not immediately obvious when a type
should be added to the initiated types tree, or even that a VM is
required to maintain one).

-Archie

__
Archie Cobbs  *CTO, Awarix*  http://www.awarix.com


___
Classpath-patches mailing list
Classpath-patches@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath-patches


[cp-patches] Re: RFC: Class Loader patch to record classwithinitiating class loader

2005-07-28 Thread Robert Lougher
Hi,

Jeroen Frijters  sumatra.nl> writes:

> 
> Mark Wielaard wrote:
> > On Wed, 2005-07-27 at 13:18 +0200, Jeroen Frijters wrote:
> > > While digging around the class loading issues, I discovered that we
> > > didn't record a class with the "initiating loader" [1]. This is
> > > necessary to maintain type safety in the face of buggy or malicious
> > > class loaders (or even when the contents of the directories in the
> > > classpath change).
> > 

First of all, I've been up all night finishing off a release, I'm
tired and my broken wrist is very sore, and I'm grouchy.  I'm getting
the excuses in early, because I'm really, really unhappy with this.

Yes, I realised this problem over a year ago, and fixed JamVM then.  While
I was waiting for the VM interface to support it I overlayed ClassLoader,
but stopped earlier this year because I was unable to support the latest
snapshot and CVS head doing this (for various reasons I felt unable to
propose a patch myself).

> > With this patch I am not completely sure what the semantics of
> > VMClassLoader.USE_VM_CACHE are. If it is set to true
> > registerInitiatingLoader() is called everywhere, but not for
> > ClassLoader.defineClass(). This seems to complicate the VM interface a
> > bit since it looks like the runtime can do this optimization of
> > registering the initiating class loader everywhere itself, 
> > not just with VMClassLoader.defineClass().
> 

I agree with Mark here.  I think the Class/VMClass/VMClassLoader
interface is already too complex and this makes it worse.  Class is
trying to do too much, leading to lots of micro-methods:

VMClassLoader.loadClass(...)
VMClass.loadArrayClass(...)
VMClass.initialize(...)
VMClass.forName(str)

and now:

VMClassLoader.USE_VM_CACHE
VMClassLoader.registerInitiatingLoader()

All this for basically Class.forName(str, res, cl)!

In my opinion, the logic for deciding whether to load via the bootstrap
or via a classloader, whether to load an array or a "normal" class, 
initialization, and the managing of the cache is best done in the runtime
for performance reasons.

At the least the logic should be pushed into the VM reference classes,
for VMs that want Classpath to do the work for them.  At present,
the runtime can't do it itself, without adding a measure of redundancy.

> It is already possible to keep the table completely in the VM (in fact,
> this is what IKVM does), but you cannot get rid of the call in
> Class.forName() because the VM is not involved in that process.
> 

And the reason you can't get rid of it is because the logic is in
Class.forName (namely the call to classloader.loadClass()).

Trying not to be partisan, in JamVM VMClassLoader.loadClass(...),
VMClass.loadArrayClass(...) and VMClass.forName(str) all map down
to the same native function (forName(s, r, cl)), which does the
logic of Class.forName(s, r, cl) itself, and records initiating
loaders.  It was done this way before the VM interface matured, and
I've preferred to keep it.

It has the advantage that loadClass on a class loader will not be
called if the VM can tell from its cache that the class loader has
already defined the class, or is marked as an initiating loader
for it.

Flame away,

Rob.

> Regards,
> Jeroen
> 




___
Classpath-patches mailing list
Classpath-patches@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath-patches