On 2020-05-13 20:31, Alan Bateman wrote:
On 13/05/2020 16:44, Claes Redestad wrote:
Hi,

compilers can't see though and optimize away the repeated GetObjectField
calls in places such as in the io_util_md.h GET_FD macro:

#define GET_FD(this, fid) \
    (*env)->GetObjectField(env, (this), (fid)) == NULL ? \
        -1 : (*env)->GetIntField(env, (*env)->GetObjectField(env, (this), (fid)), IO_fd_fdID)

This can be avoided if we turn the macros into functions which call
GetObjectField only once:

FD getFD(JNIEnv *env, jobject this, jfieldID fid) {
  jobject fd = (*env)->GetObjectField(env, this, fid);
  if (fd == NULL) {
    return -1;
  }
  return (*env)->GetIntField(env, fd, IO_fd_fdID);
}

The similarly affected SET_FD is only used in one place, so I removed
that macro and applied the equivalent optimization at the call-site.
The Windows implementation returns a HANDLE rather than an int fd so there is a bit inconsistency when compared to the network code. This pre-dates your change and technical debt that will need attention some time. Your changes look good.

Right, FD is defined as jlong on Windows, int elsewhere. Since it's used
from shared code renaming it to HANDLE on Windows variant wouldn't work
- rather we'd need some type name that would capture the duality
(FD_OR_HANDLE?)

Thanks for the review!

/Claes

Reply via email to