On 06/15/2015 08:25 AM, Mark Thomas wrote:
On 15/06/2015 06:55, jean-frederic clere wrote:
Hi,

The netty-tcnative is based on the 1.1.x so the porting taking more than
excepted.

Are you going to be able to commit this in multiple commits or is it
going to be one big commit?

A big commit it probably more easy, I have a huge diff that I am reviewing, but I will try to cut in piece like ssl.c (and depencencies), sslcontext.c etc (if possible).

Picking the netty-tcnative commits one by one would take a lot more time. (And https://github.com/netty/netty-tcnative/commit/c788f64138946f15b8fb8f53d067ccf0fb9699af is a big commit).


I would like some comments on the class caching for performances like in
ssl.c:
+++
     TCN_FREE_CSTRING(engine);
+
+    /* Cache the byte[].class for performance reasons */
+    clazz = (*e)->FindClass(e, "[B");
+    byteArrayClass = (jclass) (*e)->NewGlobalRef(e, clazz);
+
+    /* Cache the String.class for performance reasons */
+    sClazz = (*e)->FindClass(e, "java/lang/String");
+    stringClass = (jclass) (*e)->NewGlobalRef(e, sClazz);
+
      return (jint)APR_SUCCESS;
  }
+++

Should I just put or have a flag in configure for it?

I can't see a reason this would need to be optional. Either it helps
performance (in which case it should be in) or it is an unnecessary
optimisation (in which case it shouldn't be in at all).

I took a quick look at ssl.c and I didn't see a lot of lookups for those
classes? Does the netty code add some or am I missing something?

It seems they are used in etPeerCertChain() and getCiphers() only, if someone uses client certificates than might make sense.

Cheers

Jean-Frederic

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to