[icedtea-web] RFC: Patch to fix PR895
dbhole at redhat.com
Mon Mar 12 13:56:16 PDT 2012
* 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.
> >> 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..
More information about the distro-pkg-dev