NeuronRobotics / nrjavaserial

A Java Serial Port system. This is a fork of the RXTX project that uses in jar loading of the native code.
Other
345 stars 143 forks source link

short select() timeout causes MonitorThread CPU consumption #22

Closed berndpfrommer closed 8 years ago

berndpfrommer commented 9 years ago

using nrjavaserial-3.8.8, I'm seeing significant CPU consumption on low-end (atom) CPUs due to the RXTX MonitorThread being busy, although no EventListeners are attached to the port. I believe this is due to the eventLoop() in SerialImp.c having a fairly short timeout on the select() call:

                    do {
            eis.ret = SELECT( eis.fd + 1, &eis.rfds, NULL, NULL,
                &eis.tv_sleep );
        } while (eis.ret < 0 && errno==EINTR);

The eis.tv_sleep is set to only 1000usec in intialise_event_info_struct():

if (eis->fd < FD_SETSIZE && eis->fd > 0) {
    FD_SET( eis->fd, &eis->rfds );
    eis->tv_sleep.tv_sec = 0;
    eis->tv_sleep.tv_usec = 1000;
    eis->initialised = 1;
    return( 1 );
} else {
    // you can reduce this limitation only with migration to epool or something like that.
}

Which should cause the thread to wake up 1000 times per second, although no listener is attached.

Any suggestions on how to fix this?

madhephaestus commented 9 years ago

I inherited the core RXTX and this is not code I wrote. That said, i usually poll for information at the Java level and never start the event loop.

If you have a pull request to fix this, i would be happy to merge it in. We are working on a Husdson build server to generate releases from tags at the moment, and if you wand this fixed In the next major release that can be arranged.

MrDOS commented 9 years ago

This was also raised in #6, although no solution was proposed. If you have one, I'd love to hear it: such a short sleep is clearly not ideal.

berndpfrommer commented 9 years ago

Embarrassing that I didn't see #6 :( For me the cleanest solution would be to not start the monitoring thread in the first place if no event listeners are present. A bit scary to do since (at least in the original RXTX code) the read() functions tested for the presence of the monitoring thread, and would essentially abort without it running. Escapes me why that is so, but changing it has the potential to break other code out there.

Plus it wouldn't help in the scenario when listeners are attached. My first thought was to introduce another configurable parameter and let the user set the timeout on the select() call. Opinions on that?

berndpfrommer commented 9 years ago

I had to realize that any change to the native interface would need a complete recompilation on all platforms. I have no access to most of them, so that is out of the question.

acamilo commented 9 years ago

What are the platforms, i'll see if i can scrounge up some devices. Other then stuff that can be easily run on the PC the rest could probably be compiled on a pi or a gumztix

On Tue, Oct 14, 2014 at 4:54 PM, Bernd Pfrommer notifications@github.com wrote:

I had to realize that any change to the native interface would need a complete recompilation on all platforms. I have no access to most of them, so that is out of the question.

— Reply to this email directly or view it on GitHub https://github.com/NeuronRobotics/nrjavaserial/issues/22#issuecomment-59114793 .

berndpfrommer commented 9 years ago

Windows + the following. I have only ubuntu x86_64. The other question is what jni.h header file to use. Oracle JDK 1.7?

./linux/ARM_A8/libNRJavaSerial_legacy.so ./linux/ARM_A8/libNRJavaSerial.so ./linux/PPC/libNRJavaSerial.so ./linux/ARM/libNRJavaSerialv6.so ./linux/ARM/libNRJavaSerialv5.so ./linux/ARM/libNRJavaSerial_HF.so ./linux/ARM/libNRJavaSerialv6_HF.so ./linux/ARM/libNRJavaSerial.so ./linux/x86_64/libNRJavaSerial.so ./linux/x86_32/libNRJavaSerial_legacy.so

acamilo commented 9 years ago

On Tue, Oct 14, 2014 at 7:13 PM, Bernd Pfrommer notifications@github.com wrote:

Windows + the following. I have only ubuntu x86_64. The other question is what jni.h header file to use. Oracle JDK 1.7?

./linux/ARM_A8/libNRJavaSerial_legacy.so ./linux/ARM_A8/libNRJavaSerial.so

Gumstix, beaglebone have A8s, We got these (v7-A)

./linux/PPC/libNRJavaSerial.so

I have a PPC Dev board that runs Linux, no idea how hard the environment would be to set up.

./linux/ARM/libNRJavaSerialv6.so ./linux/ARM/libNRJavaSerialv5.so ./linux/ARM/libNRJavaSerial_HF.so ./linux/ARM/libNRJavaSerialv6_HF.so

v5 is covered by the chumby hacker board. v6 is the raspberry pi, both of witch we have.

./linux/ARM/libNRJavaSerial.so

?

./linux/x86_64/libNRJavaSerial.so ./linux/x86_32/libNRJavaSerial_legacy.so

x86 linux. This could be a VM

Alternatively, we could try cross compilation, or using qemu to emulate the arms, run linux, and run a toolchain. This seams less troublesome then having a bunch of dev boards in a rack somewhere.

— Reply to this email directly or view it on GitHub https://github.com/NeuronRobotics/nrjavaserial/issues/22#issuecomment-59132878 .

MrDOS commented 9 years ago

The other question is what jni.h header file to use. Oracle JDK 1.7?

21 asked us to use JDK 1.5, but the headers are the same across the board.

I've put some work in on a branch in my fork to tidy up and document the makefile and the dependencies required for cross-compilation on Ubuntu. It includes platform-dependent Java headers but you'll still need the rest of a 1.5 JDK. It builds for all platforms right now (or did, last time I checked), although I haven't actually tested any of the resulting binaries. I also still use the system glibc; I haven't yet pursued building against older glibcs.

Come to think of it, I haven't set up builds for A8 yet.

berndpfrommer commented 9 years ago

I finished a test implementation and decided that I didn't like widening the interface. In order to use the extensions one has to cast from SerialPort to RXTXPort. Plus no longer compatible with standard RXTX. Looked into configuring with a properties file, but that is ugly, too.

How about just bumping the hard coded sleep time from 1,000usec -> 100,000usec? I think the original RXTX library uses 100,000usec as polling frequency. Still 10 times per sec, but should calm things down quite a bit.

berndpfrommer commented 9 years ago

A simple workaround for the monitoring thread (if not using the event listener, but doing good old blocking read): attach an event listener and sleep indefinitely in the event handler. The blocking reads() and standard write() seems to work just fine despite the eventLoop being trapped in the handler. Makes you wonder why it has to be started in the first place...

berndpfrommer commented 8 years ago

closing this since a workaround exists.

MrDOS commented 8 years ago

@berndpfrommer We've recently incremented the sleep time to 100ms as suggested and it worked out quite well (#36). There's a pull request open (#48) with the new binaries included in its branch. If you're still involved in any projects using NRJavaSerial, we'd really appreciate it if you could confirm that they behave as expected. Either way, thanks for digging as deeply into this as you did.