Re: debug output problem in ObjectInputStream
Per Bothner wrote:
> Having dumpElement[ln] as separate
> methods has the advantage that it could over overridden, but
> it seems like poinles generality. So if we want to keep the
> debugging printout, I suggest:
>
> if (Configuration.DEBUG && dump)
> for (int i=0, len=Array.getLength(array); i < len; i++)
> System.out.println(" ELEMENT[" + i + "]=" + Array.get(array, i));
>
> This makes it possible for a compiler to do the optimization to
> multiple print calls that my patch does.
I was originally hoping that the dumpElement() methods would get inlined
and the compiler would figure out that the code could be removed, but
you're right in that even if inlining worked properly the arguments
would still have to be evaluated unless it could also figure out that
the StringBuffer methods etc had no side-effects.
I agree that we should put the "if (Configuration.DEBUG ..." at the call
site to fix this. However, fixing this doesn't really help our extremely
crap serialization performance. eg the attached test runs more than 45X
slower with GCJ than JDK 1.4! Serialization needs some work...
regards
Bryce.
import java.util.*;
import java.io.*;
public class SerTest3
{
static void showUsage()
{
System.err.println("Usage: SerTest3 ");
System.exit(1);
}
public static void main(String[] args)
throws IOException, FileNotFoundException, ClassNotFoundException
{
if (args.length == 0)
showUsage();
HashMap hm = null;
long start = System.currentTimeMillis();
if (args[0].equals("write"))
{
hm = makeHugeHashMap();
System.out.println ("map created, writing it out...");
ObjectOutputStream out =
new ObjectOutputStream(new FileOutputStream("test.ser"));
out.writeObject(hm);
out.close();
}
else if (args[0].equals("read"))
{
ObjectInputStream in =
new ObjectInputStream(new FileInputStream("test.ser"));
hm = (HashMap) in.readObject();
}
else
showUsage();
System.out.println(hm.hashCode());
System.out.println(System.currentTimeMillis() - start + "ms elapsed.");
}
static HashMap makeHugeHashMap()
{
HashMap hm = new HashMap();
for (int i=0; i < 200; i++)
{
ArrayList al = new ArrayList();
for (int j=0; j < 1000; j++)
{
al.add (Integer.toString(i - j));
}
Integer integer = new Integer(i);
hm.put(integer, al);
}
return hm;
}
}
Re: debug output problem in ObjectInputStream
> "Per" == Per Bothner <[EMAIL PROTECTED]> writes:
Per> So if we want to keep the debugging printout, I suggest:
Per>if (Configuration.DEBUG && dump)
Per> for (int i=0, len=Array.getLength(array); i < len; i++)
Per>System.out.println(" ELEMENT[" + i + "]=" + Array.get(array, i));
As I recall, Warren wrote all the debugging code. He's probably in
the best position to say how useful it really is. I've CC'd him.
(Warren, I'll forward you Per's note separately.)
Tom
___
Classpath mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/classpath
debug output problem in ObjectInputStream
(This problem was reported to me by L. Peter Deutsch.)
The class java/io/ObjectInputStream.java contains a lot
of calls to dumpElement and dumpElementln, which prints
the argument is debugging is enabled. By default the
DEBUG flag is false, so debugging is disabled. Ideally
a compiler will be able to inline and eliminate the
calls to dumpElement[ln].
The problem is that the argument to dumpElement[ln] is
still evaluated, even when DEBUG is false. In many cases
these arguments are non-trivial string concatenation expressions,
so we will get a temporary StringBuffer allocated, appended
to and thrown away. A smart compiler might be able to
figure out that the result of the concatenation is unused,
and so avoid it. Even worse though is that it calls
toString for each element of an array (and also for exceptions).
Since toString of an unknown class can do anything, the
compiler can't eliminate the call to toString. It is even
possible that toString for a large (or cyclic) data structure
could do a large(or even an infinite) amount of work.
I've appended one fix, but I'm not sure what the best approach is.
The simplest would be to just rip out all the dumpElement code.
Next simplest is to move all the Configuration.DEBUG && dump
tests out of dumpElement[ln] and to the call sites. One could
even do what my patches does, and convert the string concatenations
to multiple print calls. Having dumpElement[ln] as separate
methods has the advantage that it could over overridden, but
it seems like poinles generality. So if we want to keep the
debugging printout, I suggest:
if (Configuration.DEBUG && dump)
for (int i=0, len=Array.getLength(array); i < len; i++)
System.out.println(" ELEMENT[" + i + "]=" + Array.get(array, i));
This makes it possible for a compiler to do the optimization to
multiple print calls that my patch does.
Alteratively, I suggest just removing all the dumpElement calls.
--
--Per Bothner
[EMAIL PROTECTED] http://www.bothner.com/per/
Index: ObjectInputStream.java
===
RCS file: /cvs/gcc/gcc/libjava/java/io/ObjectInputStream.java,v
retrieving revision 1.11
diff -u -r1.11 ObjectInputStream.java
--- ObjectInputStream.java 2002/01/22 22:40:14 1.11
+++ ObjectInputStream.java 2002/04/02 20:45:49
@@ -243,8 +243,16 @@
Object array = Array.newInstance (componentType, length);
int handle = assignNewHandle (array);
readArrayElements (array, componentType);
- for (int i=0, len=Array.getLength(array); i < len; i++)
- dumpElementln (" ELEMENT[" + i + "]=" + Array.get(array, i).toString());
+ if (Configuration.DEBUG && dump)
+ {
+ for (int i=0, len=Array.getLength(array); i < len; i++)
+ {
+ System.out.print(" ELEMENT[");
+ System.out.print(i);
+ System.out.print("]=");
+ System.out.println(Array.get(array, i));
+ }
+ }
ret_val = processResolution (array, handle);
break;
}
Re: [Kissme-general] SystemClassLoader fix
John Leuner wrote: > > > I cleaned your patch up a bit, and once I get it to compile, I'm > > committing it. Thanks for the catch - I documented that the class > > specified by java.system.class.loader must now have a constructor which > > accepts a parent class loader. > > It wasn't really a patch, just a demonstration of the problem. > > Any reason why it wouldn't compile? Yes - the patch as it appeared in my reply to your email was hastily written and untested (ie. I wrote the patch and replied to your email before starting the time-consuming compilation and regression testing process). I think the only change I had to make before committing it, however, was adding an import statement. And it has been in CVS for nearly a week now. -- This signature intentionally left boring. Eric Blake [EMAIL PROTECTED] BYU student, free software programmer ___ Classpath mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/classpath
Re: [Kissme-general] SystemClassLoader fix
> I cleaned your patch up a bit, and once I get it to compile, I'm
> committing it. Thanks for the catch - I documented that the class
> specified by java.system.class.loader must now have a constructor which
> accepts a parent class loader.
It wasn't really a patch, just a demonstration of the problem.
Any reason why it wouldn't compile?
John Leuner
> John Leuner wrote:
> >
> > I tracked this down to the ClassLoader class.
> >
> >
> > Otherwise getSystemClassLoader gets in an infinite loop where the Constructor for
>SystemClassLoader calls the ClassLoader() constructor which tries to use the system
>class loader as the parent classloader and round and round it goes ...
> >
> > John Leuner
> >
>
> 2002-03-28 John Leuner <[EMAIL PROTECTED]>
>
> * java/lang/ClassLoader.java (getSystemClassLoader): Break
> infinite loop by specifying parent classloader.
> * gnu/java/lang/SystemClassLoader.java (SystemClassLoader): Add
> proper constructor.
>
> --
> This signature intentionally left boring.
>
> Eric Blake [EMAIL PROTECTED]
> BYU student, free software programmer
> Index: gnu/java/lang/SystemClassLoader.java
> ===
> RCS file: /cvsroot/classpath/classpath/gnu/java/lang/SystemClassLoader.java,v
> retrieving revision 1.1
> diff -u -r1.1 SystemClassLoader.java
> --- gnu/java/lang/SystemClassLoader.java 22 Mar 2002 21:25:20 - 1.1
> +++ gnu/java/lang/SystemClassLoader.java 28 Mar 2002 21:36:04 -
> @@ -64,6 +64,17 @@
>
>/** Flag to avoid infinite loops. */
>private static boolean is_trying;
> +
> + /**
> + * Creates a class loader. Note that the parent may be null, when this is
> + * created as the system class loader by ClassLoader.getSystemClassLoader.
> + *
> + * @param parent the parent class loader
> + */
> + public SystemClassLoader(ClassLoader parent)
> + {
> +super(parent);
> + }
>
>/**
> * Get the URL to a resource.
> Index: java/lang/ClassLoader.java
> ===
> RCS file: /cvsroot/classpath/classpath/java/lang/ClassLoader.java,v
> retrieving revision 1.16
> diff -u -r1.16 ClassLoader.java
> --- java/lang/ClassLoader.java22 Mar 2002 21:25:20 - 1.16
> +++ java/lang/ClassLoader.java28 Mar 2002 21:36:05 -
> @@ -48,6 +48,7 @@
> import java.util.Enumeration;
> import java.util.HashMap;
> import java.util.Map;
> +import gnu.java.lang.SystemClassLoader;
> import gnu.java.util.DoubleEnumeration;
> import gnu.java.util.EmptyEnumeration;
>
> @@ -685,7 +686,8 @@
> * property java.class.path. This is set as the context
> * class loader for a thread. The system property
> * java.system.class.loader, if defined, is taken to be the
> - * name of the class to use as the system class loader, otherwise this
> + * name of the class to use as the system class loader, which must have
> + * a public constructor which takes a ClassLoader as a parent; otherwise this
> * uses gnu.java.lang.SystemClassLoader.
> *
> * Note that this is different from the bootstrap classloader that
> @@ -713,12 +715,28 @@
> "gnu.java.lang.SystemClassLoader");
> try
>{
> -return (ClassLoader) Class.forName(loader).newInstance();
> +// Give the new system class loader a null parent.
> +Constructor c = Class.forName(loader).getConstructor
> + ( new Class[] { ClassLoader.class } );
> +return (ClassLoader) c.newInstance(new Object[1]);
>}
> catch (Exception e)
>{
> -throw (Error) new InternalError
> - ("System class loader could not be found: " + e).initCause(e);
> +try
> + {
> +System.err.println("Requested system classloader "
> + + loader + " failed, trying "
> + + "gnu.java.lang.SystemClassLoader");
> +e.printStackTrace();
> +// Fallback to gnu.java.lang.SystemClassLoader.
> +return new SystemClassLoader(null);
> + }
> +catch (Exception e1)
> + {
> +throw (Error) new InternalError
> + ("System class loader could not be found: " + e)
> + .initCause(e);
> + }
>}
>}
> // Check if we may return the system classloader
___
Classpath mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/classpath
