[icedtea-web] RFC: Patch to fix PR895

[Deepak Bhole]

* Omair Majid <omajid at redhat.com> [2012-03-12 21:42]:
> On 03/12/2012 04:56 PM, Deepak Bhole wrote:
> > * 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.
> 
> Thanks!
> 
> >>>> 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.
> > 
> 
> Perhaps it might be better to wait then (at least for HEAD) ?
> 

Yeah, definitely.

> > 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..
> 
> See attached patch (fails without your patch; passes with it). It just
> tests timing; I can extend it to check functionality too.
>

Ah, yeah I thought of checking for time too but I was hoping to avoid it
as it is not reliable. In any case, given that neither of us have a
better way in mind, I guess this is better than nothing :)
 
> As a side note, we should really try and reduce the inter-dependencies
> (between the classloader, jnlpfile and downloading), but this works for now.
> 

Yeah :/ there have been other cases where this has been a hindrance to
writing tests too.

Are you OK with the content of the patch? If so, I will commit it and
your test -- thanks!

Deepak