hadoop-common-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sameer Paranjpye (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HADOOP-538) Implement a nio's 'direct buffer' based wrapper over zlib to improve performance of java.util.zip.{De|In}flater as a 'custom codec'
Date Thu, 26 Oct 2006 19:26:49 GMT
    [ http://issues.apache.org/jira/browse/HADOOP-538?page=comments#action_12444950 ] 
            
Sameer Paranjpye commented on HADOOP-538:
-----------------------------------------

Some more comments on the native code:

Makefiles etc.

- We should compile with '-Wall'. 
- What about 64-bit vs 32-bit builds, this makefile will generate the OS default, which may
be 
incompatible with the JVM used. For instance on a 64-bit machine with a 32-bit JVM this will
generate
a 64-bit library, which will be incompatible with the JVM. We should check the JVM version
and compile
with -m32 or -m64 as appropriate.

-----------

Index: src/native/org/apache/hadoop/io/compress/zlib/ZlibCompressor.c

+JNIEXPORT jlong JNICALL
+Java_org_apache_hadoop_io_compress_zlib_ZlibCompressor_init(
+	JNIEnv *env, jclass class, jint level, jint strategy
+	) {
+	// Create a z_stream
+    z_stream *stream = malloc(sizeof(z_stream));
+    if (!stream) {
+		THROW(env, "java/lang/OutOfMemoryError", NULL);
+		return (jlong)0;
+    } 
+    memset((void*)stream, 0, sizeof(z_stream));
...
...
...
+
+    return (jlong)((int)stream);
+}
+

Is this the only way to return a void* to the JVM? This is a bad, unsafe cast, not 64-bit
safe. 
If a cast like this is needed it should be '(jlong)(stream)'.


+JNIEXPORT jint JNICALL
+Java_org_apache_hadoop_io_compress_zlib_ZlibCompressor_deflateBytesDirect(
+	JNIEnv *env, jobject this
+	) {
+	// Get members of ZlibCompressor
+    z_stream *stream = (z_stream *)
+  				((int)(*env)->GetLongField(env, this, ZlibCompressor_stream));
...
...
...
+}

Cast to int is not 64-bit safe.


-------------
Index: src/native/org/apache/hadoop/io/compress/zlib/ZlibDecompressor.c


+JNIEXPORT jlong JNICALL
+Java_org_apache_hadoop_io_compress_zlib_ZlibDecompressor_init(
+	JNIEnv *env, jclass cls
+	) {
+    z_stream *stream = malloc(sizeof(z_stream));
+    memset((void*)stream, 0, sizeof(z_stream));
+
...
...
...
+	return (jint)((int)stream);
+}


Cast is not 64-bit safe.



-----------------
Index: src/native/org/apache/hadoop/io/compress/zlib/org_apache_hadoop_io_compress_zlib_util.h

+// TODO: Fix the below macro for 64-bit systems
+// Helpful macro to convert a jlong to z_stream*
+#define ZSTREAM(stream) ((z_stream*)((int)stream))


Act on the comment, please. :)



-----------------
Index: src/native/org_apache_hadoop.h

+ 
+ #define THROW(env, exception_name, message) \
+ 			{ \
+			 	jclass ecls = (*env)->FindClass(env, exception_name); \
+			 	if (ecls) { \
+					(*env)->ThrowNew(env, ecls, message); \
+		 		} \
+ 			}
+		 			 			


This macro creates a local reference to a class in jclass and does not destroy it, causing
a memory leak.
Add a '(*env)->DestroyLocalReference(env, ecls)' statement.



> Implement a nio's 'direct buffer' based wrapper over zlib to improve performance of java.util.zip.{De|In}flater
as a 'custom codec'
> -----------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HADOOP-538
>                 URL: http://issues.apache.org/jira/browse/HADOOP-538
>             Project: Hadoop
>          Issue Type: Improvement
>    Affects Versions: 0.6.1
>            Reporter: Arun C Murthy
>         Assigned To: Arun C Murthy
>             Fix For: 0.8.0
>
>         Attachments: HADOOP-538.patch, HADOOP-538_20061005.tgz, HADOOP-538_20061011.tgz,
HADOOP-538_20061026.tgz, HADOOP-538_benchmarks.tgz
>
>
> There has been more than one instance where java.util.zip's {De|In}flater classes perform
unreliably, a simple wrapper over zlib-1.2.3 (latest stable) using java.nio.ByteBuffer (i.e.
direct buffers) should go a long way in alleviating these woes.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message