Re: [cp-patches] RFC: Object de-/serialization improvement

2006-08-21 Thread Roman Kennke

Hi there,

Jeroen Frijters schrieb:

Roman Kennke wrote:

What about

|| (l == VMClassLoader.getSystemClassLoader())

instead ?


You shouldn't call VMClassLoader.getSystemClassLoader() (it creates a
new instance of the system class loader), but apart from that comparing
against the system class loader would work, but I'd much rather see a
solution that allows caching for other classes as well.


So what should we do about it now? Should I check in the optimization 
with this comparison fixed to use the system classloader for now? And 
think about improving this afterwards?



I really would
like to explore the option of adding a field to ClassLoader a little
more (and hear other people's opinions about that).


Me too.

/Roman



Re: [cp-patches] RFC: File/VMFile improvement

2006-08-21 Thread Roman Kennke

Hi there again,

What about this pending patch? I didn't quite understand the discussion 
around it. IMO the file handling is very platform dependend and while we 
could handle this generically, I find this path very inconvenient, as it 
clutters the code, where it could be easy, and it certainly isn't very 
good for efficiency. I agree that this is not necessarily VM dependent, 
but instead platform dependend? But I don't think that we should 
introduce some kind of platform-layer that complements the VM layer in 
some way. As Jeroen already pointed out, often times the VM is somewhat 
tied to the platform(s) it supports and thus can just as well provide a 
reasonable implementation of file handling.


As for the VMBlahSocketImpl, I would very much like to have this mess 
cleaned up (I know, I introduced this in the first place). I came to 
think of that we don't really need this anyway, as the specs already 
have a way of abstracting this stuff out (by defining the SocketImpl 
abstract classes) and we simply added another unnecessary layer here.


So may I commit this change to the VMFile class for now? Or should we go 
another way of splitting out platform dependend stuff?


/Roman

Roman Kennke schrieb:

Jeroen Frijters schrieb:

Hi Roman,

The attached patch file is empty.


Ugh. My bad. Here it is.

/Roman




Index: vm/reference/java/io/VMFile.java
===
RCS file: /cvsroot/classpath/classpath/vm/reference/java/io/VMFile.java,v
retrieving revision 1.8
diff -u -1 -2 -r1.8 VMFile.java
--- vm/reference/java/io/VMFile.java7 Jun 2006 15:09:40 -   1.8
+++ vm/reference/java/io/VMFile.java15 Aug 2006 15:41:49 -
@@ -29,24 +29,27 @@
 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 java.io;
 
+import java.net.MalformedURLException;

+import java.net.URL;
+
 import gnu.classpath.Configuration;
 import gnu.java.io.PlatformHelper;
 
 
 /**

  * @author Michael Koch ([EMAIL PROTECTED])
  */
 final class VMFile
 {
   // FIXME: We support only case sensitive filesystems currently.
   static final boolean IS_CASE_SENSITIVE = true;
   static final boolean IS_DOS_8_3 = false;
@@ -200,24 +203,126 @@
   {
int pos = PlatformHelper.lastIndexOfSeparator(path);
if (pos == -1)
  return path;

if (PlatformHelper.endWithSeparator(path))
  return ;

return path.substring(pos + File.separator.length());
   }
 
   /**

+   * Returns the path as an absolute path name. The value returned is the
+   * current directory plus the separatory string plus the path of the file.
+   * The current directory is determined from the codeuser.dir/code system
+   * property.
+   *
+   * @param path the path to convert to absolute path
+   *
+   * @return the absolute path that corresponds to codepath/code
+   */
+  static String getAbsolutePath(String path)
+  {
+if (File.separatorChar == '\\' 
+   path.length()  0  path.charAt (0) == '\\')

+  {
+// On Windows, even if the path starts with a '\\' it is not
+// really absolute until we prefix the drive specifier from
+// the current working directory to it.
+return System.getProperty (user.dir).substring (0, 2) + path;
+  }
+else if (File.separatorChar == '\\'
+  path.length()  1  path.charAt (1) == ':'
+  ((path.charAt (0) = 'a'  path.charAt (0) = 'z')
+ || (path.charAt (0) = 'A'  path.charAt (0) = 'Z')))
+  {
+// On Windows, a process has a current working directory for
+// each drive and a path like G:foo\bar would mean the 
+// absolute path G:\wombat\foo\bar if \wombat is the 
+// working directory on the G drive.

+String drvDir = null;
+try
+  {
+drvDir = new File (path.substring (0, 2)).getCanonicalPath();
+  }
+catch (IOException e)
+  {
+drvDir = path.substring (0, 2) + \\;
+  }
+
+// Note: this would return C:\\. for the path C:., if \
+// is the working folder on the C drive, but this is 
+// consistent with what Sun's JRE 1.4.1.01 actually returns!

+if (path.length()  2)
+  return drvDir + '\\' + path.substring (2, path.length());
+else
+  return drvDir;
+  }
+else if (path.equals())
+  return System.getProperty (user.dir);
+

RE: [cp-patches] RFC: Object de-/serialization improvement

2006-08-21 Thread Jeroen Frijters
Roman Kennke wrote:
 Jeroen Frijters schrieb:
  Roman Kennke wrote:
  What about
 
  || (l == VMClassLoader.getSystemClassLoader())
 
  instead ?
  
  You shouldn't call VMClassLoader.getSystemClassLoader() (it 
 creates a
  new instance of the system class loader), but apart from 
 that comparing
  against the system class loader would work, but I'd much 
 rather see a
  solution that allows caching for other classes as well.
 
 So what should we do about it now? Should I check in the optimization 
 with this comparison fixed to use the system classloader for now? And 
 think about improving this afterwards?

Sure.

  I really would
  like to explore the option of adding a field to ClassLoader a little
  more (and hear other people's opinions about that).
 
 Me too.

I'll cook up a proposal.

Regards,
Jeroen



RE: [cp-patches] RFC: File/VMFile improvement

2006-08-21 Thread Jeroen Frijters
Roman Kennke wrote:
 What about this pending patch?

I certainly would like to see it go.

Regards,
Jeroen



Re: [cp-patches] RFC: File/VMFile improvement

2006-08-21 Thread Roman Kennke

Hi,

Jeroen Frijters schrieb:

Roman Kennke wrote:

What about this pending patch?


I certainly would like to see it go.


Ok. Dalibor also expressed that he'd like to see it in. So here it goes.

2006-08-21  Roman Kennke  [EMAIL PROTECTED]

* java/io/File.java
(getAbsolutePath): Fetch absolute path from
VMFile.getAbsolutePath(). Moved actual impl to there.
(isAbsolute): Let VMFile determine the absoluteness.
(toURL): Let VMFile convert the filename.
* vm/reference/java/io/VMFile.java
(getAbsolutePath): New method.
(isAbsolute): New method.
(toURL): New method.

/Roman
Index: java/io/File.java
===
RCS file: /cvsroot/classpath/classpath/java/io/File.java,v
retrieving revision 1.65
diff -u -1 -2 -r1.65 File.java
--- java/io/File.java	29 Jun 2006 09:59:57 -	1.65
+++ java/io/File.java	21 Aug 2006 09:21:14 -
@@ -418,63 +418,26 @@
* This method returns the path of this file as an absolute path name.
* If the path name is already absolute, then it is returned.  Otherwise
* the value returned is the current directory plus the separatory
* string plus the path of the file.  The current directory is determined
* from the codeuser.dir/code system property.
*
* @return The absolute path of this file
*/
   public String getAbsolutePath()
   {
 if (isAbsolute())
   return path;
-else if (separatorChar == '\\' 
-  path.length()  0  path.charAt (0) == '\\')
-  {
-// On Windows, even if the path starts with a '\\' it is not
-// really absolute until we prefix the drive specifier from
-// the current working directory to it.
-return System.getProperty (user.dir).substring (0, 2) + path;
-  }
-else if (separatorChar == '\\' 
-  path.length()  1  path.charAt (1) == ':'
-  ((path.charAt (0) = 'a'  path.charAt (0) = 'z')
- || (path.charAt (0) = 'A'  path.charAt (0) = 'Z')))
-  {
-// On Windows, a process has a current working directory for
-// each drive and a path like G:foo\bar would mean the 
-// absolute path G:\wombat\foo\bar if \wombat is the 
-// working directory on the G drive.
-String drvDir = null;
-try
-  {
-drvDir = new File (path.substring (0, 2)).getCanonicalPath();
-  }
-catch (IOException e)
-  {
-drvDir = path.substring (0, 2) + \\;
-  }
-
-// Note: this would return C:\\. for the path C:., if \
-// is the working folder on the C drive, but this is 
-// consistent with what Sun's JRE 1.4.1.01 actually returns!
-if (path.length()  2)
-  return drvDir + '\\' + path.substring (2, path.length());
-else
-  return drvDir;
-  }
-else if (path.equals())
-  return System.getProperty (user.dir);
 else
-  return System.getProperty (user.dir) + separatorChar + path;
+  return VMFile.getAbsolutePath(path);
   }
 
   /**
* This method returns a codeFile/code object representing the
* absolute path of this object.
*
* @return A codeFile/code with the absolute path of the object.
*
* @since 1.2
*/
   public File getAbsoluteFile()
   {
@@ -648,33 +611,25 @@
 
   /**
* This method returns true if this object represents an absolute file
* path and false if it does not.  The definition of an absolute path varies
* by system.  As an example, on GNU systems, a path is absolute if it starts
* with a /.
*
* @return codetrue/code if this object represents an absolute 
* file name, codefalse/code otherwise.
*/
   public boolean isAbsolute()
   {
-if (separatorChar == '\\')
-	return path.startsWith(dupSeparator) || 
-	(path.length()  2  
-	 ((path.charAt(0) = 'a'  path.charAt(0) = 'z') ||
-	  (path.charAt(0) = 'A'  path.charAt(0) = 'Z')) 
-	 path.charAt(1) == ':' 
-	 path.charAt(2) == '\\');
-else
-	return path.startsWith(separator);
+return VMFile.isAbsolute(path);
   }
 
   /**
* This method tests whether or not the file represented by this object
* is a directory.  In order for this method to return codetrue/code,
* the file represented by this object must exist and be a directory.
* 
* @return codetrue/code if this file is a directory, codefalse/code
* otherwise
*
* @exception SecurityException If reading of the file is not permitted
*/
@@ -989,32 +944,25 @@
   /**
* This method returns a codeURL/code with the codefile:/code
* protocol that represents this file.  The exact form of this URL is
* system dependent.
*
* @return A codeURL/code for this object.
*
* @exception MalformedURLException If the URL cannot be created 
* successfully.
*/
   public URL toURL() throws MalformedURLException
 

[cp-patches] FYI: InputStreamReader improvement

2006-08-21 Thread Roman Kennke
I'm committing this on behalf of Ingo. He implemented a caching 
mechanism in InputStreamReader to avoid allocation of new byte[] on 
every call of read(byte[],int,int).


2006-08-21  Ingo Proetel  [EMAIL PROTECTED]

* java/io/InputStreamReader.java
(bytesCache): New field.
(cacheLock): New field.
(read(byte[],int,int): Avoid allocations of new byte
array on every call and reuse cached byte array if possible.

/Roman
Index: java/io/InputStreamReader.java
===
RCS file: /cvsroot/classpath/classpath/java/io/InputStreamReader.java,v
retrieving revision 1.29
diff -u -1 -2 -r1.29 InputStreamReader.java
--- java/io/InputStreamReader.java	12 Feb 2006 09:36:21 -	1.29
+++ java/io/InputStreamReader.java	21 Aug 2006 10:00:34 -
@@ -126,24 +126,34 @@
* java.io canonical name of the encoding.
*/
   private String encoding;
 
   /**
* We might decode to a 2-char UTF-16 surrogate, which won't fit in the
* output buffer. In this case we need to save the surrogate char.
*/
   private char savedSurrogate;
   private boolean hasSavedSurrogate = false;
 
   /**
+   * A byte array to be reused in read(byte[], int, int).
+   */
+  private byte[] bytesCache;
+
+  /**
+   * Locks the bytesCache above in read(byte[], int, int).
+   */
+  private Object cacheLock = new Object();
+
+  /**
* This method initializes a new instance of codeInputStreamReader/code
* to read from the specified stream using the default encoding.
*
* @param in The codeInputStream/code to read from 
*/
   public InputStreamReader(InputStream in)
   {
 if (in == null)
   throw new NullPointerException();
 this.in = in;
 try 
 	{ 
@@ -346,27 +356,37 @@
* @param length The requested number of characters to read.
*
* @return The actual number of characters read, or -1 if end of stream.
*
* @exception IOException If an error occurs
*/
   public int read(char[] buf, int offset, int length) throws IOException
   {
 if (in == null)
   throw new IOException(Reader has been closed);
 if (isDone)
   return -1;
-if(decoder != null){
-	int totalBytes = (int)((double)length * maxBytesPerChar);
-	byte[] bytes = new byte[totalBytes];
+if(decoder != null)
+  {
+	int totalBytes = (int)((double) length * maxBytesPerChar);
+	byte[] bytes;
+// Fetch cached bytes array if available and big enough.
+synchronized(cacheLock)
+  {
+bytes = bytesCache;
+if (bytes == null || bytes.length  totalBytes)
+  bytes = new byte[totalBytes];
+else
+  bytesCache = null;
+  }
 
 	int remaining = 0;
 	if(byteBuffer != null)
 	{
 	remaining = byteBuffer.remaining();
 	byteBuffer.get(bytes, 0, remaining);
 	}
 	int read;
 	if(totalBytes - remaining  0)
 	  {
 	read = in.read(bytes, remaining, totalBytes - remaining);
 	if(read == -1){
@@ -401,30 +421,58 @@
 	  isDone = false;
 	}
 	}
 
 	if(byteBuffer.hasRemaining()) {
 	byteBuffer.compact();
 	byteBuffer.flip();	  
 	isDone = false;
 	} else
 	byteBuffer = null;
 
 	read = cb.position() - startPos;
-	return (read = 0) ? -1 : read;
-} else {
-	byte[] bytes = new byte[length];
+
+// Put cached bytes array back if we are finished and the cache
+// is null or smaller than the used bytes array.
+synchronized (cacheLock)
+  {
+if (byteBuffer == null
+ (bytesCache == null || bytesCache.length  bytes.length))
+  bytesCache = bytes;
+  }
+return (read = 0) ? -1 : read;
+  }
+else
+  {
+	byte[] bytes;
+// Fetch cached bytes array if available and big enough.
+synchronized (cacheLock)
+  {
+bytes = bytesCache;
+if (bytes == null || length  bytes.length)
+  bytes = new byte[length];
+else
+  bytesCache = null;
+  }
+
 	int read = in.read(bytes);
 	for(int i=0;iread;i++)
   buf[offset+i] = (char)(bytes[i]0xFF);
+
+// Put back byte array into cache if appropriate.
+synchronized (cacheLock)
+  {
+if (bytesCache == null || bytesCache.length  bytes.length)
+  bytesCache = bytes;
+  }
 	return read;
 }
   }
 
   /**
* Reads an char from the input stream and returns it
* as an int in the range of 0-65535.  This method also will return -1 if
* the end of the stream has been reached.
* p
* This method will block until the char can be read.
*
* @return The char read or -1 if end of stream


[cp-patches] FYI: CSSScanner cleanup

2006-08-21 Thread Roman Kennke
This changes the CSSScanner to compare the ints directly, rather then 
casting to char everytime. Now it only casts to char when putting the 
char into the char[] buffer.


2006-08-21  Roman Kennke  [EMAIL PROTECTED]

* gnu/javax/swing/text/html/css/CSSScanner.java
(main): Use buffered input stream.
(nextToken): Removed 65536 workaround. Use int value directly
without cast to char.
(readComment): Use int value directly without cast to char.
Cast to char only when putting the character into the buffer.
(readEscape): Likewise.
(readIdent): Likewise.
(readName): Likewise.
(readNum): Likewise.
(readString): Likewise.
(readWhitespace): Likewise.

/Roman
Index: gnu/javax/swing/text/html/css/CSSScanner.java
===
RCS file: /cvsroot/classpath/classpath/gnu/javax/swing/text/html/css/CSSScanner.java,v
retrieving revision 1.2
diff -u -1 -2 -r1.2 CSSScanner.java
--- gnu/javax/swing/text/html/css/CSSScanner.java	18 Aug 2006 20:22:35 -	1.2
+++ gnu/javax/swing/text/html/css/CSSScanner.java	21 Aug 2006 10:06:56 -
@@ -29,24 +29,25 @@
 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 gnu.javax.swing.text.html.css;
 
+import java.io.BufferedInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.Reader;
 
 /**
  * A parser for CSS stylesheets. This is based on the grammar from:
  *
  * http://www.w3.org/TR/CSS21/syndata.html
  *
  * @author Roman Kennke ([EMAIL PROTECTED])
  */
@@ -112,226 +113,222 @@
* Fetches the next token. The actual character data is in the parseBuffer
* afterwards with the tokenStart at index 0 and the tokenEnd field
* pointing to the end of the token.
*
* @return the next token
*/
   int nextToken()
 throws IOException
   {
 tokenEnd = 0;
 int token = -1;
 int next = read();
-if (next == 65535)
-  System.err.println(BUG);
-if (next != -1  next != 65535) // Workaround.
+if (next != -1)
   {
-char ch = (char) next;
-switch (ch)
+switch (next)
 {
   case ';':
-parseBuffer[0] = ch;
+parseBuffer[0] = (char) next;
 tokenEnd = 1;
 token = SEMICOLON;
 break;
   case '{':
-parseBuffer[0] = ch;
+parseBuffer[0] = (char) next;
 tokenEnd = 1;
 token = CURLY_LEFT;
 break;
   case '}':
-parseBuffer[0] = ch;
+parseBuffer[0] = (char) next;
 tokenEnd = 1;
 token = CURLY_RIGHT;
 break;
   case '(':
-parseBuffer[0] = ch;
+parseBuffer[0] = (char) next;
 tokenEnd = 1;
 token = PAREN_LEFT;
 break;
   case ')':
-parseBuffer[0] = ch;
+parseBuffer[0] = (char) next;
 tokenEnd = 1;
 token = PAREN_RIGHT;
 break;
   case '[':
-parseBuffer[0] = ch;
+parseBuffer[0] = (char) next;
 tokenEnd = 1;
 token = BRACE_LEFT;
 break;
   case ']':
-parseBuffer[0] = ch;
+parseBuffer[0] = (char) next;
 tokenEnd = 1;
 token = BRACE_RIGHT;
 break;
   case '@':
-parseBuffer[0] = ch;
+parseBuffer[0] = (char) next;
 tokenEnd = 1;
 readIdent();
 token = ATKEYWORD;
 break;
   case '#':
-parseBuffer[0] = ch;
+parseBuffer[0] = (char) next;
 tokenEnd = 1;
 readName();
 token = HASH;
 break;
   case '\'':
   case '':
-lookahead[0] = ch;
+lookahead[0] = next;
 readString();
 token = STRING;
 break;
   case ' ':
   case '\t':
   case '\r':
   case '\n':
   case '\f':
-lookahead[0] = ch;
+lookahead[0] = next;
 readWhitespace();
 token = S;
 break;
 // FIXME: Detecting an URI involves several characters lookahead.
 //  case 'u':
 //lookahead[0] = ch;
 //readURI();
 //token = URI;
 //break;
   case '':

[cp-patches] FYI: Fixed java.io.File normalizePath bug

2006-08-21 Thread Jeroen Frijters
Hi,

I committed the attached patch to fix the handling of paths consisting
only of a double separator character. Our normalization would previously
turn this into an empty string, instead of a single separator character.
This happened both on Windows and on *nix.

Regards,
Jeroen

2006-08-21  Jeroen Frijters  [EMAIL PROTECTED]

* java/io/File.java (normalizePath): Fixed handling of // and
\\.
Index: java/io/File.java
===
RCS file: /cvsroot/classpath/classpath/java/io/File.java,v
retrieving revision 1.66
diff -u -r1.66 File.java
--- java/io/File.java   21 Aug 2006 09:23:01 -  1.66
+++ java/io/File.java   21 Aug 2006 12:46:42 -
@@ -286,7 +286,8 @@
 // example, is a valid and minimal path).
 if (plen  1  p.charAt (plen - 1) == separatorChar)
  {
-   if (! (separatorChar == '\\'  plen == 3  p.charAt (1) == ':'))
+   if (! (separatorChar == '\\'  ((plen == 3  p.charAt(1) == ':')
+|| (plen == 2  p.charAt(0) == separatorChar
  return p.substring (0, plen - 1);
  }
else
@@ -303,7 +304,16 @@
  {
dupIndex++;
if (dupIndex == plen)
- return newpath.toString();
+  {
+if ((separatorChar == '\\'
+ newpath.length() == 2
+ newpath.charAt(1) == ':')
+|| (separatorChar != '\\'  newpath.length() == 0))
+  {
+newpath.append(separatorChar);
+  }
+   return newpath.toString();
+  }
  }
newpath.append(separatorChar);
last = dupIndex;
@@ -315,7 +325,9 @@
 int end;
 if (plen  1  p.charAt (plen - 1) == separatorChar)
 {
-  if (separatorChar == '\\'  plen == 3  p.charAt (1) == ':')
+  if (separatorChar == '\\'
+ ((plen == 3  p.charAt(1) == ':')
+|| (plen == 2  p.charAt(0) == separatorChar)))
 end = plen;
   else
 end = plen - 1;


Re: [cp-patches] RFC: ClassLoader associative cache

2006-08-21 Thread Archie Cobbs
Jeroen Frijters wrote:
 There are several places in our codebase were we need/want to cache
 stuff that is associated with a class or class loader. Currently these
 places retain a strong reference to the class loader, which is not
 correct, because the class loader should be garbage collectable.
 
 I cooked up proposal for an API to use for this and implemented it in
 the three places I could think of off the top my head (serialization,
 proxy and resource bundle).
 
 This is just a first stab, so no documentation and no change log entry
 yet. Please comment on the idea and/or the API.

Dumb question.. why wouldn't it work to just use a WeakHashMap instead
of a HashMap in all those places?

-Archie

__
Archie Cobbs  *CTO, Awarix*  http://www.awarix.com



RE: [cp-patches] RFC: ClassLoader associative cache

2006-08-21 Thread Jeroen Frijters
Jeroen Frijters wrote:
 Archie Cobbs wrote:
  Dumb question.. why wouldn't it work to just use a 
  WeakHashMap instead of a HashMap in all those places?
 
 Not a dumb question at all. That *would* work. The problem 
 is that (at least on some runtimes) WeakHashMap is much more
 expensive than this approach.

Actually, it would not work in all cases. For serialization it would
work, because there the key is a Class instance, but for both Proxy and
ResourceBundle, the key is more complex, so it wouldn't work to directly
use a WeakHashMap (you'd have to add a layer of indirection, effectively
keeping a separate cache per class loader and in that case this approach
is arguably better).

There may be other reasons, Roman also didn't like to use a WeakHashMap,
but I didn't ask why.

Using a WeakHashMap would also make it a little harder to implement the
ResourceBundle LRU policy, but that was not the motivation for this
approach.

Regards,
Jeroen



[cp-patches] RFC: JTree and BasicToolBarUI

2006-08-21 Thread Tania Bento
Hey,

The application JabRef was not even starting on Classpath.  This patch
enables it to start up.  However, it seems to freeze once that happens.
It still needs some work.

Could someone kindly approve this patch so that I may commit it.

Thanks!
Tania

2006-08-21  Tania Bento  [EMAIL PROTECTED]

* javax/swing/plaf/basic/BasicToolBarUI.java:
(setBorderToNonRollover): If border == null, then the
insets should be used when saving the old border in the
hastable.
* javax/swing/JTree.java:
(setLeadSelectionPath): Need to make sure that path != null when
determining if path.equals(oldValue). This avoid a Null Pointer
Exception.

Index: javax/swing/JTree.java
===
RCS file: /cvsroot/classpath/classpath/javax/swing/JTree.java,v
retrieving revision 1.73
diff -u -r1.73 JTree.java
--- javax/swing/JTree.java	13 Aug 2006 20:34:03 -	1.73
+++ javax/swing/JTree.java	21 Aug 2006 18:47:10 -
@@ -2355,7 +2355,7 @@
 if (selectionModel != null)
   {
 TreePath oldValue = selectionModel.getLeadSelectionPath();
-if (path.equals(oldValue))
+if (path != null  path.equals(oldValue))
   return;

 // Repaint the previous and current rows with the lead selection path.
Index: javax/swing/plaf/basic/BasicToolBarUI.java
===
RCS file: /cvsroot/classpath/classpath/javax/swing/plaf/basic/BasicToolBarUI.java,v
retrieving revision 1.28
diff -u -r1.28 BasicToolBarUI.java
--- javax/swing/plaf/basic/BasicToolBarUI.java	26 Jul 2006 13:39:39 -	1.28
+++ javax/swing/plaf/basic/BasicToolBarUI.java	21 Aug 2006 18:47:10 -
@@ -898,7 +898,10 @@
 	b.setRolloverEnabled(false);
 
 // Save old border in hashtable.
-borders.put(b, b.getBorder());
+if (b.getBorder() == null)
+  borders.put(b, b.getInsets());
+else
+  borders.put(b, b.getBorder());
 
 	b.setBorder(nonRolloverBorder);
   }


Re: [cp-patches] FYI: LocalSocket fixlets

2006-08-21 Thread Casey Marshall
Roman Kennke wrote:
 This fixes two little issues that I had when porting the LocalSocketImpl
 to JamaicaVM:
 
 - It only loads the library dynamically when actually supported by the
 runtime.
 - It includes config.h unconditionally. Actually it doesn't seem to make
 sense to do:
 
 #if HAVE_CONFIG_H
 #include config.h
 #endif
 
 since HAVE_CONFIG_H is defined in config.h.
 

You should leave the #if around the #include (actually, it should be
#ifdef, I think). You'll notice that HAVE_CONFIG_H is defined on the gcc
command line in Classpath; autoconf/autoheader adds that to your
compile-time flags so you know when you can include config.h. If it's
biting you that HAVE_CONFIG_H isn't defined, and you need to include
config.h, you should define it, then ;-)

I think this is the autoconf way: you don't #include or use something
unless you have a HAVE_ macro telling you it's OK. I mean, you could
very well just use gcc flags at compile time *instead* of config.h; the
latter just makes it easy to have lots of conditionals. I also think
this is typical GNU practice, so please leave the conditionals in.

Thanks.



Re: [cp-patches] Merge NATIVE-LAYER to HEAD

2006-08-21 Thread Mark Wielaard
On Sun, 2006-08-20 at 18:12 -0700, Casey Marshall wrote:
  Since this finally cleans up the target layer stuff I would like this to
  go in now and then we build on this for your further improvements to NIO
  and the socket timeouts. OK?
 
 OK. Since I'm working off of cvs head I'm probably queering up the diffs
 and history; is there any issue with me overwiting some of these
 changes, not necessarily building off of them?

No, certainly not. Just do make sure to test early and often :)
For timeouts you might want to look at the new failing omg/corba tests
that seem to happen with guilhem's rewrite. Those are regressions, so we
should fix them.

 Since this cleans up stuff beyond javanet, I'd like to see it committed.

OK, committed.

Cheers,

Mark


signature.asc
Description: This is a digitally signed message part


Re: [cp-patches] RFC: JTree and BasicToolBarUI

2006-08-21 Thread Mark Wielaard
Hey Tania,

On Mon, 2006-08-21 at 15:00 -0400, Tania Bento wrote:
 The application JabRef was not even starting on Classpath.  This patch
 enables it to start up.  However, it seems to freeze once that happens.
 It still needs some work.

Cool. CCed Egon who was interested JabRef.
Could one of you update the JabRef entry on
http://developer.classpath.org/mediation/FreeSwingTestApps

 2006-08-21  Tania Bento  [EMAIL PROTECTED]
 
 * javax/swing/plaf/basic/BasicToolBarUI.java:
 (setBorderToNonRollover): If border == null, then the
 insets should be used when saving the old border in the
 hastable.
 * javax/swing/JTree.java:
 (setLeadSelectionPath): Need to make sure that path != null when
 determining if path.equals(oldValue). This avoid a Null Pointer
 Exception.

Maybe that check needs to be 
 if (path == oldValue || path != null  path.equals(oldValue))
to catch the case both path and oldValue are null?

Thanks,

Mark




Re: [cp-patches] RFC: File/VMFile improvement

2006-08-21 Thread Casey Marshall
Roman Kennke wrote:
 Hi there again,
 
 What about this pending patch? I didn't quite understand the discussion
 around it. IMO the file handling is very platform dependend and while we
 could handle this generically, I find this path very inconvenient, as it
 clutters the code, where it could be easy, and it certainly isn't very
 good for efficiency. I agree that this is not necessarily VM dependent,
 but instead platform dependend? But I don't think that we should
 introduce some kind of platform-layer that complements the VM layer in
 some way. As Jeroen already pointed out, often times the VM is somewhat
 tied to the platform(s) it supports and thus can just as well provide a
 reasonable implementation of file handling.
 

Sorry, that was kind of a bikeshed issue. VM-vs-platform isn't *that*
big a deal, it just feels clumsy, and has been used as an I don't
understand this umbrella in the past.

 As for the VMBlahSocketImpl, I would very much like to have this mess
 cleaned up (I know, I introduced this in the first place). I came to
 think of that we don't really need this anyway, as the specs already
 have a way of abstracting this stuff out (by defining the SocketImpl
 abstract classes) and we simply added another unnecessary layer here.
 

This is true, and it's the same with java.nio (it's essentially a
service provider interface through the SelectorProvider class). There is
*some* value in isolating these platform-specific bits as much as
possible (and keeping them, and the amount of JNI code, to a minimum!),
and making porting Classpath a low-barrier-to-entry.

What, if any, performance tradeoffs there are is a good question, however.

 So may I commit this change to the VMFile class for now? Or should we go
 another way of splitting out platform dependend stuff?
 

Sorry for not looking at it. The patch looks reasonable to me. Real code
is always preferable to me bickering about vaporware :-)