[icedtea-web] RFC: Patch to fix PR895

[Omair Majid]

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) ?

> 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.

As a side note, we should really try and reduce the inter-dependencies
(between the classloader, jnlpfile and downloading), but this works for now.

Cheers,
Omair
-------------- next part --------------
A non-text attachment was scrubbed...
Name: unit-test.patch
Type: text/x-patch
Size: 5904 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20120312/a3d19c6d/unit-test.patch