[icedtea-web] RFC: Patch to fix PR895

[Deepak Bhole]

* Omair Majid <omajid at redhat.com> [2012-03-12 14:32]:
> On 03/12/2012 02:20 PM, Deepak Bhole wrote:
> > * Deepak Bhole <dbhole at redhat.com> [2012-03-12 14:13]:
> >> * Omair Majid <omajid at redhat.com> [2012-03-09 20:08]:
> >>> On 03/09/2012 05:19 PM, Deepak Bhole wrote:
> >>>> Hi,
> >>>>
> >>>> Attached patch fixes PR895:
> >>>> "IcedTea-Web searches for missing classes on each loadClass or findClass"
> >>>>
> >>>> http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=895
> >>>
> >>> The bug doesn't seem to have any additional information. Is there a test
> >>> case or a reproducer? Do you know why the server is being hammered? If
> >>> the actual issue is under our control (for example, we are attempting to
> >>> re-download stuff we already downloaded), it might be better to fix this
> >>> underlying issue.
> >>>
> >>
> >> Yes, this one:
> >> http://jdk6.java.net/nonav/plugin2/liveconnect/LiveConnectTests/
> >>
> 
> Please do add this to
> http://icedtea.classpath.org/wiki/IcedTea-Web-Tests as well.
> 

Added.

> >> Without the patch it takes 2+ minutes to run.. with patch, it is down to
> >> about 15 seconds.
> >>
> 
> Do you know what this test is doing? While the patch looks fine, I would
> hate for us to paper over the problem rather than fixing it.
> 

They are a set of LiveConnect tests. From what I can tell, the js is
constantly asking the plugin to get a property named 'java', 'lang',
etc. Since one can have a class named java or lang, the plug-in searches
all available paths. The tests repeatedly ask for these paths/packages.

Looking at the code now, I think it should be possible to improve this to
not have it do it over and over though. The class lookup is done from
IcedTeaScriptableJavaPackageObject::getProperty so it shouldn't have to
look up a class with the name.

I cannot remember why I made it do it though and I would like to think
about it before fiddling with it (in case I had a reason back then). I
remember that some of the tests in the IcedTea LiveConnect suite fail
with the Oracle plug-in because it has issues with direct access to
classes/packages and I suspect this may have been the reason. I will
think about it.

> >>
> >> While writing the above, I realized that it is really just CBCL that we
> >> want to fix. What about moving the logic just into CBCL? See attached
> >> patch.
> >>
> > 
> > annndddd... here is the patch
> > 
> 
> Just one request: can you please add a unit test for this classloader so
> we can verify that this behaviour does not regress later?
> 

Sure, but how do you propose we test this behaviour with a unit test?
The return of the function is intended to be identical when class is
searched (and not found) for the first time, or subsequent ones..

Cheers,
Deepak