[icedtea-web] RFC: Patch to fix PR895
Jiri Vanek
jvanek at redhat.com
Tue Mar 13 02:19:15 PDT 2012
On 03/13/2012 02:42 AM, Omair Majid wrote:
> 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
>
>
> unit-test.patch
>
>
> diff --git a/tests/netx/unit/net/sourceforge/jnlp/runtime/CodeBaseClassLoaderTest.java b/tests/netx/unit/net/sourceforge/jnlp/runtime/CodeBaseClassLoaderTest.java
> new file mode 100644
> --- /dev/null
> +++ b/tests/netx/unit/net/sourceforge/jnlp/runtime/CodeBaseClassLoaderTest.java
> @@ -0,0 +1,142 @@
> +/* CodeBaseClassLoaderTest.java
> + Copyright (C) 2012 Red Hat, Inc.
> +
> +This file is part of IcedTea.
> +
> +IcedTea is free software; you can redistribute it and/or
> +modify it under the terms of the GNU General Public License as published by
> +the Free Software Foundation, version 2.
> +
> +IcedTea is distributed in the hope that it will be useful,
> +but WITHOUT ANY WARRANTY; without even the implied warranty of
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +General Public License for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with IcedTea; see the file COPYING. If not, write to
> +the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> +02110-1301 USA.
> +
> +Linking this library statically or dynamically with other modules is
> +making a combined work based on this library. Thus, the terms and
> +conditions of the GNU General Public License cover the whole
> +combination.
> +
> +As a special exception, the copyright holders of this library give you
> +permission to link this library with independent modules to produce an
> +executable, regardless of the license terms of these independent
> +modules, and to copy and distribute the resulting executable under
> +terms of your choice, provided that you also meet, for each linked
> +independent module, the terms and conditions of the license of that
> +module. An independent module is a module which is not derived from
> +or based on this library. If you modify this library, you may extend
> +this exception to your version of the library, but you are not
> +obligated to do so. If you do not wish to do so, delete this
> +exception statement from your version.
> +*/
> +
> +package net.sourceforge.jnlp.runtime;
> +
> +import static org.junit.Assert.assertTrue;
> +
> +import java.io.IOException;
> +import java.net.URL;
> +import java.util.Locale;
> +
> +import net.sourceforge.jnlp.JNLPFile;
> +import net.sourceforge.jnlp.LaunchException;
> +import net.sourceforge.jnlp.ParseException;
> +import net.sourceforge.jnlp.ResourcesDesc;
> +import net.sourceforge.jnlp.SecurityDesc;
> +import net.sourceforge.jnlp.runtime.JNLPClassLoader;
> +import net.sourceforge.jnlp.runtime.JNLPClassLoader.CodeBaseClassLoader;
> +
> +import org.junit.Test;
> +
> +public class CodeBaseClassLoaderTest {
> +
> + @Test
> + public void testResourceLoadSuccessCaching() throws LaunchException, ClassNotFoundException, IOException, ParseException {
> + final URL JAR_URL = new URL("http://icedtea.classpath.org/netx/about.jar");
> + final URL CODEBASE_URL = new URL("http://icedtea.classpath.org/netx/");
> +
> + JNLPFile dummyJnlpFile = new JNLPFile() {
> + @Override
> + public ResourcesDesc getResources() {
> + return new ResourcesDesc(null, new Locale[0], new String[0], new String[0]);
> + }
> +
> + @Override
> + public URL getCodeBase() {
> + return CODEBASE_URL;
> + }
> +
> + @Override
> + public SecurityDesc getSecurity() {
> + return new SecurityDesc(null, SecurityDesc.SANDBOX_PERMISSIONS, null);
> + }
> + };
> +
> + JNLPClassLoader parent = new JNLPClassLoader(dummyJnlpFile, null);
> + CodeBaseClassLoader classLoader = new CodeBaseClassLoader(new URL[] { JAR_URL, CODEBASE_URL }, parent);
> +
> + long startTime, stopTime;
> +
> + startTime = System.nanoTime();
> + classLoader.findResource("net/sourceforge/jnlp/about/Main.class");
> + stopTime = System.nanoTime();
> + long timeOnFirstTry = stopTime - startTime;
> + System.out.println(timeOnFirstTry);
> +
> + startTime = System.nanoTime();
> + classLoader.findResource("net/sourceforge/jnlp/about/Main.class");
> + stopTime = System.nanoTime();
> + long timeOnSecondTry = stopTime - startTime;
> + System.out.println(timeOnSecondTry);
> +
> + assertTrue(timeOnSecondTry< (timeOnFirstTry / 10));
> + }
> +
> + @Test
> + public void testResourceLoadFailureCaching() throws LaunchException, ClassNotFoundException, IOException, ParseException {
> + final URL JAR_URL = new URL("http://icedtea.classpath.org/netx/about.jar");
> + final URL CODEBASE_URL = new URL("http://icedtea.classpath.org/netx/");
> +
> + JNLPFile dummyJnlpFile = new JNLPFile() {
> + @Override
> + public ResourcesDesc getResources() {
> + return new ResourcesDesc(null, new Locale[0], new String[0], new String[0]);
> + }
> +
> + @Override
> + public URL getCodeBase() {
> + return CODEBASE_URL;
> + }
> +
> + @Override
> + public SecurityDesc getSecurity() {
> + return new SecurityDesc(null, SecurityDesc.SANDBOX_PERMISSIONS, null);
> + }
> + };
> +
> + JNLPClassLoader parent = new JNLPClassLoader(dummyJnlpFile, null);
> + CodeBaseClassLoader classLoader = new CodeBaseClassLoader(new URL[] { JAR_URL, CODEBASE_URL }, parent);
> +
> + long startTime, stopTime;
> +
> + startTime = System.nanoTime();
> + classLoader.findResource("net/sourceforge/jnlp/about/Main_FOO_.class");
> + stopTime = System.nanoTime();
> + long timeOnFirstTry = stopTime - startTime;
> + System.out.println(timeOnFirstTry);
> +
> + startTime = System.nanoTime();
> + classLoader.findResource("net/sourceforge/jnlp/about/Main_FOO_.class");
> + stopTime = System.nanoTime();
> + long timeOnSecondTry = stopTime - startTime;
> + System.out.println(timeOnSecondTry);
> +
> + assertTrue(timeOnSecondTry< (timeOnFirstTry / 10));
> + }
> +
> +}
>
What is causing the delay? The jar is downloaded again? In case that yes, you can use local server with slowed downloading (Pavel is just reviewing this change [1]) and so get rid of remote intent connection's requirement during unittests. (you can direct the internal server to local build of about.jar so it will remain unittest)
J.
[1] thread Applet's in browser, slow loading, remote and (in some bright future) graphical testing and some minor improvements (was Re: Icedtea-web splashscreen implementation)
More information about the distro-pkg-dev
mailing list