[RFA/JDWP] VMVirtualMachine StringBuffer cleanup

[RFA/JDWP] VMVirtualMachine StringBuffer cleanup

[Keith Seitz]

[-- Attachment #1: Type: text/plain, Size: 566 bytes --]

Hi,

I've been meaning to cleanup a lot of the gnu/classpath/jdwp/nat*.cc 
files, and this is one of the things that made it onto my pet peeve list 
recently. The attached patch switches to the 
gnu.gcj.runtime.StringBuffer instead of the java.lang one.

I've got some other little cleanups that I'll submit after this.

Keith

ChangeLog
2006-11-08  Keith Seitz  <keiths@redhat.com>

         * gnu/classpath/jdwp/natVMVirtualMachine.cc (suspendThread): Use
         gnu.gcj.runtime.StringBuffer instead of java.lang.StringBuffer.
         (resumeThread): Likewise.

[-- Attachment #2: vmvirtualmachine-stringbuffer.patch --]
[-- Type: text/x-patch, Size: 2284 bytes --]

Index: gnu/classpath/jdwp/natVMVirtualMachine.cc
===================================================================
--- gnu/classpath/jdwp/natVMVirtualMachine.cc	(revision 118009)
+++ gnu/classpath/jdwp/natVMVirtualMachine.cc	(working copy)
@@ -17,7 +17,6 @@
 #include <java/lang/ClassLoader.h>
 #include <java/lang/Integer.h>
 #include <java/lang/String.h>
-#include <java/lang/StringBuffer.h>
 #include <java/lang/Thread.h>
 #include <java/nio/ByteBuffer.h>
 #include <java/util/ArrayList.h>
@@ -30,6 +29,7 @@
 #include <gnu/classpath/jdwp/event/EventRequest.h>
 #include <gnu/classpath/jdwp/exception/JdwpInternalErrorException.h>
 #include <gnu/classpath/jdwp/util/MethodResult.h>
+#include <gnu/gcj/runtime/StringBuffer.h>
 
 using namespace java::lang;
 using namespace gnu::classpath::jdwp::event;
@@ -75,13 +75,12 @@
       jvmtiError err = _jdwp_jvmtiEnv->SuspendThread (thread);
       if (err != JVMTI_ERROR_NONE)
 	{
+	  using namespace gnu::gcj::runtime;
 	  using namespace gnu::classpath::jdwp::exception;
 	  char *reason;
 	  _jdwp_jvmtiEnv->GetErrorName (err, &reason);
-	  ::java::lang::String *txt
-	      = JvNewStringLatin1 ("could not suspend thread: ");
-	  ::java::lang::StringBuffer *msg
-	      = new ::java::lang::StringBuffer (txt);
+	  String *txt = JvNewStringLatin1 ("could not suspend thread: ");
+	  StringBuffer *msg = new StringBuffer (txt);
 	  msg->append (JvNewStringLatin1 (reason));
 	  _jdwp_jvmtiEnv->Deallocate ((unsigned char *) reason);
 	  throw new JdwpInternalErrorException (msg->toString ());
@@ -126,13 +125,12 @@
       jvmtiError err = _jdwp_jvmtiEnv->ResumeThread (thread);
       if (err != JVMTI_ERROR_NONE)
 	{
+	  using namespace gnu::gcj::runtime;
 	  using namespace gnu::classpath::jdwp::exception;
 	  char *reason;
 	  _jdwp_jvmtiEnv->GetErrorName (err, &reason);
-	  ::java::lang::String *txt 
-	      = JvNewStringLatin1 ("could not resume thread: ");
-	  ::java::lang::StringBuffer *msg
-	      = new ::java::lang::StringBuffer (txt);
+	  String *txt = JvNewStringLatin1 ("could not resume thread: ");
+	  StringBuffer *msg = new StringBuffer (txt);
 	  msg->append (JvNewStringLatin1 (reason));
 	  _jdwp_jvmtiEnv->Deallocate ((unsigned char *) reason);
 	  throw new JdwpInternalErrorException (msg->toString ());

Re: [RFA/JDWP] VMVirtualMachine StringBuffer cleanup

[Andrew Haley]

Keith Seitz writes:
 > Hi,
 > 
 > I've been meaning to cleanup a lot of the gnu/classpath/jdwp/nat*.cc 
 > files, and this is one of the things that made it onto my pet peeve list 
 > recently. The attached patch switches to the 
 > gnu.gcj.runtime.StringBuffer instead of the java.lang one.
 > 
 > I've got some other little cleanups that I'll submit after this.

I'm doing a big merge from trunk -> branch.  Please don't commit
anything to trun ATM.

Andrew.

Re: [RFA/JDWP] VMVirtualMachine StringBuffer cleanup

[Keith Seitz]

Post-merge ping.

Keith

Keith Seitz wrote:
> Hi,
> 
> I've been meaning to cleanup a lot of the gnu/classpath/jdwp/nat*.cc 
> files, and this is one of the things that made it onto my pet peeve list 
> recently. The attached patch switches to the 
> gnu.gcj.runtime.StringBuffer instead of the java.lang one.
> 
> I've got some other little cleanups that I'll submit after this.
> 
> Keith
> 
> ChangeLog
> 2006-11-08  Keith Seitz  <keiths@redhat.com>
> 
>         * gnu/classpath/jdwp/natVMVirtualMachine.cc (suspendThread): Use
>         gnu.gcj.runtime.StringBuffer instead of java.lang.StringBuffer.
>         (resumeThread): Likewise.
> 
> 
> ------------------------------------------------------------------------
> 
> Index: gnu/classpath/jdwp/natVMVirtualMachine.cc
> ===================================================================
> --- gnu/classpath/jdwp/natVMVirtualMachine.cc	(revision 118009)
> +++ gnu/classpath/jdwp/natVMVirtualMachine.cc	(working copy)
> @@ -17,7 +17,6 @@
>  #include <java/lang/ClassLoader.h>
>  #include <java/lang/Integer.h>
>  #include <java/lang/String.h>
> -#include <java/lang/StringBuffer.h>
>  #include <java/lang/Thread.h>
>  #include <java/nio/ByteBuffer.h>
>  #include <java/util/ArrayList.h>
> @@ -30,6 +29,7 @@
>  #include <gnu/classpath/jdwp/event/EventRequest.h>
>  #include <gnu/classpath/jdwp/exception/JdwpInternalErrorException.h>
>  #include <gnu/classpath/jdwp/util/MethodResult.h>
> +#include <gnu/gcj/runtime/StringBuffer.h>
>  
>  using namespace java::lang;
>  using namespace gnu::classpath::jdwp::event;
> @@ -75,13 +75,12 @@
>        jvmtiError err = _jdwp_jvmtiEnv->SuspendThread (thread);
>        if (err != JVMTI_ERROR_NONE)
>  	{
> +	  using namespace gnu::gcj::runtime;
>  	  using namespace gnu::classpath::jdwp::exception;
>  	  char *reason;
>  	  _jdwp_jvmtiEnv->GetErrorName (err, &reason);
> -	  ::java::lang::String *txt
> -	      = JvNewStringLatin1 ("could not suspend thread: ");
> -	  ::java::lang::StringBuffer *msg
> -	      = new ::java::lang::StringBuffer (txt);
> +	  String *txt = JvNewStringLatin1 ("could not suspend thread: ");
> +	  StringBuffer *msg = new StringBuffer (txt);
>  	  msg->append (JvNewStringLatin1 (reason));
>  	  _jdwp_jvmtiEnv->Deallocate ((unsigned char *) reason);
>  	  throw new JdwpInternalErrorException (msg->toString ());
> @@ -126,13 +125,12 @@
>        jvmtiError err = _jdwp_jvmtiEnv->ResumeThread (thread);
>        if (err != JVMTI_ERROR_NONE)
>  	{
> +	  using namespace gnu::gcj::runtime;
>  	  using namespace gnu::classpath::jdwp::exception;
>  	  char *reason;
>  	  _jdwp_jvmtiEnv->GetErrorName (err, &reason);
> -	  ::java::lang::String *txt 
> -	      = JvNewStringLatin1 ("could not resume thread: ");
> -	  ::java::lang::StringBuffer *msg
> -	      = new ::java::lang::StringBuffer (txt);
> +	  String *txt = JvNewStringLatin1 ("could not resume thread: ");
> +	  StringBuffer *msg = new StringBuffer (txt);
>  	  msg->append (JvNewStringLatin1 (reason));
>  	  _jdwp_jvmtiEnv->Deallocate ((unsigned char *) reason);
>  	  throw new JdwpInternalErrorException (msg->toString ());

Re: [RFA/JDWP] VMVirtualMachine StringBuffer cleanup

[Andrew Haley]

Keith Seitz writes:
 > Post-merge ping.
 > 
 > Keith
 > 
 > Keith Seitz wrote:
 > > Hi,
 > > 
 > > I've been meaning to cleanup a lot of the gnu/classpath/jdwp/nat*.cc 
 > > files, and this is one of the things that made it onto my pet peeve list 
 > > recently. The attached patch switches to the 
 > > gnu.gcj.runtime.StringBuffer instead of the java.lang one.
 > > 
 > > I've got some other little cleanups that I'll submit after this.
 > > 
 > > Keith
 > > 
 > > ChangeLog
 > > 2006-11-08  Keith Seitz  <keiths@redhat.com>
 > > 
 > >         * gnu/classpath/jdwp/natVMVirtualMachine.cc (suspendThread): Use
 > >         gnu.gcj.runtime.StringBuffer instead of java.lang.StringBuffer.
 > >         (resumeThread): Likewise.

Use java.lang.StringBuilder instead of gnu.gcj.runtime.StringBuffer.
Patch approved with that change.

Andrew.

Re: [RFA/JDWP] VMVirtualMachine StringBuffer cleanup

[Andrew Haley]

Andrew Haley writes:
 > Keith Seitz writes:
 >  > Post-merge ping.
 >  > 
 >  > Keith
 >  > 
 >  > Keith Seitz wrote:
 >  > > Hi,
 >  > > 
 >  > > I've been meaning to cleanup a lot of the gnu/classpath/jdwp/nat*.cc 
 >  > > files, and this is one of the things that made it onto my pet peeve list 
 >  > > recently. The attached patch switches to the 
 >  > > gnu.gcj.runtime.StringBuffer instead of the java.lang one.
 >  > > 
 >  > > I've got some other little cleanups that I'll submit after this.
 >  > > 
 >  > > Keith
 >  > > 
 >  > > ChangeLog
 >  > > 2006-11-08  Keith Seitz  <keiths@redhat.com>
 >  > > 
 >  > >         * gnu/classpath/jdwp/natVMVirtualMachine.cc (suspendThread): Use
 >  > >         gnu.gcj.runtime.StringBuffer instead of java.lang.StringBuffer.
 >  > >         (resumeThread): Likewise.
 > 
 > Use java.lang.StringBuilder instead of gnu.gcj.runtime.StringBuffer.
 > Patch approved with that change.

Forget that.  Use gnu.gcj.runtime.StringBuffer if you wish.

Andrew.

Re: [RFA/JDWP] VMVirtualMachine StringBuffer cleanup

[Keith Seitz]

Andrew Haley wrote:

>  > Use java.lang.StringBuilder instead of gnu.gcj.runtime.StringBuffer.
>  > Patch approved with that change.
> 
> Forget that.  Use gnu.gcj.runtime.StringBuffer if you wish.

Is there a preference? Since this is another case of using one of these 
things to build a String once without ever modifying it. Is 
g.g.r.StringBuffer the preferred way to do this, or am I confusing it 
with something else?

Keith

Re: [RFA/JDWP] VMVirtualMachine StringBuffer cleanup

[Andrew Haley]

Keith Seitz writes:
 > Andrew Haley wrote:
 > 
 > >  > Use java.lang.StringBuilder instead of gnu.gcj.runtime.StringBuffer.
 > >  > Patch approved with that change.
 > > 
 > > Forget that.  Use gnu.gcj.runtime.StringBuffer if you wish.
 > 
 > Is there a preference? Since this is another case of using one of these 
 > things to build a String once without ever modifying it. Is 
 > g.g.r.StringBuffer the preferred way to do this, or am I confusing it 
 > with something else?

gnu.gcj.runtime.StringBuffer is more efficient, but
java.lang.StringBuilder is standard.  My preference is for standard
where possible, but sometimes efficiency wins.  My complaint was
therefore inappropriate.

Andrew.

Re: [RFA/JDWP] VMVirtualMachine StringBuffer cleanup

[Keith Seitz]

Andrew Haley wrote:

> Patch approved with that change.

Okay, I don't really see too much of a difference between 
gnu.gcj.runtime.StringBuffer and java.lang.StringBuilder, so I opted for 
the latter.

I have committed this patch. Thanks for taking a peek at it.

Keith

Re: [RFA/JDWP] VMVirtualMachine StringBuffer cleanup

[Tom Tromey]

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> Okay, I don't really see too much of a difference between
Keith> gnu.gcj.runtime.StringBuffer and java.lang.StringBuilder, so I opted
Keith> for the latter.

The difference is that StringBuilder has to make a copy of the char[]
data when toString is called.  Our StringBuffer shares the data, which
is more efficient.

However, this only makes a difference if you expect this code to be
run with some frequency.

Tom