java-gnome-hackers

[java-gnome-hackers] String changes

[Mark H. <mh...@ti...>]

Hi,
  As discussed at the start of the year, 
  I'd like to make some big changes to the way we handle passing of
strings. The current code has major issues on some (non-Sun) Java
environments (when working with zero length strings) and also does not
take into account charset encoding. 

These changes:
 * use  (*env)->GetStringUTFChars rather than 
        jint str_len = (*env)->GetArrayLength(env, str);
        gchar* str_g = (gchar*)g_malloc(str_len + 1);
        (*env)->GetByteArrayRegion(env, str, 0, str_len, (jbyte*)str_g);
        str_g[str_len] = 0;
 * Also call    (*env)->ReleaseStringUTFChars(env, str, str_g);
   - As far as I can see, the old code is wrong in that it should have
been calling ReleaseByteArrayElements when done 
   - please let me know if you don't agree with this.


The attached files demonstrate the changes I propose. Test.c shows both
the current and proposed methods
They can be executed using 
gcc -c -g -O2 -fPIC Test.c -o Test.o -I/usr/include/gtk-2.0
-I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0
-I/usr/include/pango-1.0 -I/usr/include/freetype2
-I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include

   gcc   -g -O2 -fPIC -shared -o libTest.so Test.o -Wl,--export-dynamic
-lgtk-x11-2.0 -lgdk-x11-2.0 -latk-1.0 -lgdk_pixbuf-2.0 -lm
-lpangoxft-1.0 -lpangox-1.0 -lpango-1.0 -lgobject-2.0 -lgmodule-2.0 -ldl
-lglib-2.0 -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include
-I/usr/include/atk-1.0 -I/usr/include/pango-1.0 -I/usr/include/freetype2
-I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -Xlinker
--no-undefined

javac TestClass.java
LD_LIBRARY_PATH=. java TestClass

In my tests, the new code always also has improved performance (Java
1.4.1)

The changes would require regenerating all the generated code and
(hopefully semi-automatically) changing all calls to these.

Any comments?
-- 
  .''`. Mark Howard
 : :' :
 `. `'  http://www.tildemh.com 
   `-   mh...@de... | mh...@ti... | mh...@ca... 

[java-gnome-hackers] String changes

[Mark H. <mh...@ti...>]

Hi,
  As discussed at the start of the year, 
  I'd like to make some big changes to the way we handle passing of
strings. The current code has major issues on some (non-Sun) Java
environments (when working with zero length strings) and also does not
take into account charset encoding. 

These changes:
 * use  (*env)->GetStringUTFChars rather than 
	jint str_len = (*env)->GetArrayLength(env, str);
        gchar* str_g = (gchar*)g_malloc(str_len + 1);
        (*env)->GetByteArrayRegion(env, str, 0, str_len, (jbyte*)str_g);
        str_g[str_len] = 0;
 * Also call 	(*env)->ReleaseStringUTFChars(env, str, str_g);
   - As far as I can see, the old code is wrong in that it should have
been calling ReleaseByteArrayElements when done 
   - please let me know if you don't agree with this.


The attached files demonstrate the changes I propose. Test.c shows both
the current and proposed methods
They can be executed using 
gcc -c -g -O2 -fPIC Test.c -o Test.o -I/usr/include/gtk-2.0
-I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0
-I/usr/include/pango-1.0 -I/usr/include/freetype2
-I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include

   gcc   -g -O2 -fPIC -shared -o libTest.so Test.o -Wl,--export-dynamic
-lgtk-x11-2.0 -lgdk-x11-2.0 -latk-1.0 -lgdk_pixbuf-2.0 -lm
-lpangoxft-1.0 -lpangox-1.0 -lpango-1.0 -lgobject-2.0 -lgmodule-2.0 -ldl
-lglib-2.0 -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include
-I/usr/include/atk-1.0 -I/usr/include/pango-1.0 -I/usr/include/freetype2
-I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -Xlinker
--no-undefined

javac TestClass.java
LD_LIBRARY_PATH=. java TestClass

In my tests, the new code always also has improved performance (Java
1.4.1)

The changes would require regenerating all the generated code and
(hopefully semi-automatically) changing all calls to these.

Any comments?
-- 
  .''`. Mark Howard
 : :' :
 `. `'  http://www.tildemh.com 
   `-   mh...@de... | mh...@ti... | mh...@ca... 

Re: [java-gnome-hackers] String changes

[Tom B. <Tom...@Su...>]

The way strings are handled has been bothering me as well, most recently
in finding that repeatedly calling TextBuffer.setText leaks native
memory in a manner I haven't figured out yet.  To clean it up, I
modified the code in the same manner Mark described.  

Your point about not supporting charset encoding is an excellent one --
GNOME more than many projects is sensitive to internationalization
issues, so we can't ignore this problem and stay in step with GNOME. 
Since Java and GTK already share the same internal String format, we
will reduce the code overhead and improve compatibility with this
change.  At JavaSoft we used to call that sort of enhancement request a
"no-brainer", in that it was a change even a clueless exec would
approve. :-)

Like Jeffrey, I'm nervous about regenerating the existing source files. 
By all means update the generator so that new files are created
correctly, but most of your conversion impacts native code rather than
the generated Java code.  Since most of the conversion has to be done by
hand any way, let's do it all by hand so each class can be converted
individually to minimize breakage.  Doing it piecemeal keeps the source
base functional without anyone having to pull an all-nighter.

To avoid duplication of effort, how about if we notify others before
converting any major classes?  I have TextBuffer half-done already, so
unless there are objections, I'll finish it and put it back today.

Tom

On Wed, 2003-03-19 at 13:22, Mark Howard wrote:
> Hi,
>   As discussed at the start of the year, 
>   I'd like to make some big changes to the way we handle passing of
> strings. The current code has major issues on some (non-Sun) Java
> environments (when working with zero length strings) and also does not
> take into account charset encoding. 
> 
> These changes:
>  * use  (*env)->GetStringUTFChars rather than 
>         jint str_len = (*env)->GetArrayLength(env, str);
>         gchar* str_g = (gchar*)g_malloc(str_len + 1);
>         (*env)->GetByteArrayRegion(env, str, 0, str_len, (jbyte*)str_g);
>         str_g[str_len] = 0;
>  * Also call    (*env)->ReleaseStringUTFChars(env, str, str_g);
>    - As far as I can see, the old code is wrong in that it should have
> been calling ReleaseByteArrayElements when done 
>    - please let me know if you don't agree with this.
> 
> 
> The attached files demonstrate the changes I propose. Test.c shows both
> the current and proposed methods
> They can be executed using 
> gcc -c -g -O2 -fPIC Test.c -o Test.o -I/usr/include/gtk-2.0
> -I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0
> -I/usr/include/pango-1.0 -I/usr/include/freetype2
> -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include
> 
>    gcc   -g -O2 -fPIC -shared -o libTest.so Test.o -Wl,--export-dynamic
> -lgtk-x11-2.0 -lgdk-x11-2.0 -latk-1.0 -lgdk_pixbuf-2.0 -lm
> -lpangoxft-1.0 -lpangox-1.0 -lpango-1.0 -lgobject-2.0 -lgmodule-2.0 -ldl
> -lglib-2.0 -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include
> -I/usr/include/atk-1.0 -I/usr/include/pango-1.0 -I/usr/include/freetype2
> -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -Xlinker
> --no-undefined
> 
> javac TestClass.java
> LD_LIBRARY_PATH=. java TestClass
> 
> In my tests, the new code always also has improved performance (Java
> 1.4.1)
> 
> The changes would require regenerating all the generated code and
> (hopefully semi-automatically) changing all calls to these.
> 
> Any comments?
-- 
Tom Ball <Tom...@Su...>