Request for review: 7166048: remove the embedded epoll data structure
Charles Lee
littlee at linux.vnet.ibm.com
Mon May 7 01:48:31 PDT 2012
Hi Sean,
The patch is committed @
Changeset: 62557a1336c0
Author: zhouyx
Date: 2012-05-07 16:43 +0800
URL:http://hg.openjdk.java.net/jdk8/tl/jdk/rev/62557a1336c0
7166048: Remove the embeded epoll data structure.
Reviewed-by: alanb
Please verify it. And thanks Alan to review it.
On 05/07/2012 03:36 PM, Sean Chou wrote:
> Hi Alan,
>
> The new patch is at:
> http://cr.openjdk.java.net/~zhouyx/7166048/webrev.03/
> <http://cr.openjdk.java.net/%7Ezhouyx/7166048/webrev.03/> . There are
> some spaces difference between the old and new file. Please take a
> look again.
>
> On Fri, May 4, 2012 at 3:59 PM, Alan Bateman <Alan.Bateman at oracle.com
> <mailto:Alan.Bateman at oracle.com>> wrote:
>
> On 04/05/2012 08:35, Sean Chou wrote:
>
> Hi all,
>
> I found the
> src/solaris/native/sun/nio/ch/EPollArrayWrapper.c embedded the
> epoll data structure epoll_data and epoll_event . It is
> duplicated with the definition in sys/epoll.h . This
> redundancy would cause failure if the underlying system has
> different epoll data structure (eg. different alignment) .
>
> I reported 7166048 for it and made a patch:
> http://cr.openjdk.java.net/~zhouyx/7166048/webrev.00/
> <http://cr.openjdk.java.net/%7Ezhouyx/7166048/webrev.00/>
> <http://cr.openjdk.java.net/%7Ezhouyx/7166048/webrev.00/> .
>
>
> As background, when the epoll Selector was added then the JDK had
> to be build on distributions that were still 2.4 based and didn't
> have epoll.h. We had meant to go back and clear this up in JDK7
> but didn't get around to it. So thanks for taking this on now. I
> looked at your patch but it doesn't seem to be complete as it
> doesn't change the usages to call the epoll functions directly.
> Attached is the patch that I had for cleaning this up, it should
> still be current because this code has not changed. The only thing
> I notice is that it doesn't remove the include of dlfcn.h, that
> shouldn't be needed now.
>
> On the RFE that you submitted, I've moved this to the right place as:
>
> 7166048: (se) EPollArrayWrapper.c no longer needs to define epoll
> data structures
>
> -Alan.
>
>
>
> diff --git a/src/solaris/native/sun/nio/ch/EPollArrayWrapper.c
> b/src/solaris/native/sun/nio/ch/EPollArrayWrapper.c
> --- a/src/solaris/native/sun/nio/ch/EPollArrayWrapper.c
> +++ b/src/solaris/native/sun/nio/ch/EPollArrayWrapper.c
> @@ -34,55 +34,13 @@
> #include <unistd.h>
> #include <sys/resource.h>
> #include <sys/time.h>
> -
> -#ifdef __cplusplus
> -extern "C" {
> -#endif
> -
> -/* epoll_wait(2) man page */
> -
> -typedef union epoll_data {
> - void *ptr;
> - int fd;
> - __uint32_t u32;
> - __uint64_t u64;
> -} epoll_data_t;
> -
> -
> -/* x86-64 has same alignment as 32-bit */
> -#ifdef __x86_64__
> -#define EPOLL_PACKED __attribute__((packed))
> -#else
> -#define EPOLL_PACKED
> -#endif
> -
> -struct epoll_event {
> - __uint32_t events; /* Epoll events */
> - epoll_data_t data; /* User data variable */
> -} EPOLL_PACKED;
> -
> -#ifdef __cplusplus
> -}
> -#endif
> +#include <sys/epoll.h>
>
> #define RESTARTABLE(_cmd, _result) do { \
> do { \
> _result = _cmd; \
> } while((_result == -1) && (errno == EINTR)); \
> } while(0)
> -
> -/*
> - * epoll event notification is new in 2.6 kernel. As the offical
> build
> - * platform for the JDK is on a 2.4-based distribution then we must
> - * obtain the addresses of the epoll functions dynamically.
> - */
> -typedef int (*epoll_create_t)(int size);
> -typedef int (*epoll_ctl_t) (int epfd, int op, int fd, struct
> epoll_event *event);
> -typedef int (*epoll_wait_t) (int epfd, struct epoll_event
> *events, int maxevents, int timeout);
> -
> -static epoll_create_t epoll_create_func;
> -static epoll_ctl_t epoll_ctl_func;
> -static epoll_wait_t epoll_wait_func;
>
> static int
> iepoll(int epfd, struct epoll_event *events, int numfds, jlong
> timeout)
> @@ -96,7 +54,7 @@ iepoll(int epfd, struct epoll_event *eve
> start = t.tv_sec * 1000 + t.tv_usec / 1000;
>
> for (;;) {
> - int res = (*epoll_wait_func)(epfd, events, numfds, timeout);
> + int res = epoll_wait(epfd, events, numfds, timeout);
> if (res < 0 && errno == EINTR) {
> if (remaining >= 0) {
> gettimeofday(&t, NULL);
> @@ -117,14 +75,6 @@ JNIEXPORT void JNICALL
> JNIEXPORT void JNICALL
> Java_sun_nio_ch_EPollArrayWrapper_init(JNIEnv *env, jclass this)
> {
> - epoll_create_func = (epoll_create_t) dlsym(RTLD_DEFAULT,
> "epoll_create");
> - epoll_ctl_func = (epoll_ctl_t) dlsym(RTLD_DEFAULT,
> "epoll_ctl");
> - epoll_wait_func = (epoll_wait_t) dlsym(RTLD_DEFAULT,
> "epoll_wait");
> -
> - if ((epoll_create_func == NULL) || (epoll_ctl_func == NULL) ||
> - (epoll_wait_func == NULL)) {
> - JNU_ThrowInternalError(env, "unable to get address of
> epoll functions, pre-2.6 kernel?");
> - }
> }
>
> JNIEXPORT jint JNICALL
> @@ -134,7 +84,7 @@ Java_sun_nio_ch_EPollArrayWrapper_epollC
> * epoll_create expects a size as a hint to the kernel about
> how to
> * dimension internal structures. We can't predict the size in
> advance.
> */
> - int epfd = (*epoll_create_func)(256);
> + int epfd = epoll_create(256);
> if (epfd < 0) {
> JNU_ThrowIOExceptionWithLastError(env, "epoll_create failed");
> }
> @@ -173,7 +123,7 @@ Java_sun_nio_ch_EPollArrayWrapper_epollC
> event.events = events;
> event.data.fd = fd;
>
> - RESTARTABLE((*epoll_ctl_func)(epfd, (int)opcode, (int)fd,
> &event), res);
> + RESTARTABLE(epoll_ctl(epfd, (int)opcode, (int)fd, &event), res);
>
> /*
> * A channel may be registered with several Selectors. When
> each Selector
> @@ -199,7 +149,7 @@ Java_sun_nio_ch_EPollArrayWrapper_epollW
> int res;
>
> if (timeout <= 0) { /* Indefinite or no wait */
> - RESTARTABLE((*epoll_wait_func)(epfd, events, numfds,
> timeout), res);
> + RESTARTABLE(epoll_wait(epfd, events, numfds, timeout), res);
> } else { /* Bounded wait; bounded restarts */
> res = iepoll(epfd, events, numfds, timeout);
> }
>
>
>
>
> --
> Best Regards,
> Sean Chou
>
--
Yours Charles
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20120507/c9af9092/attachment-0001.html
More information about the nio-dev
mailing list