New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GPIOs too slow - RaspiGpioProvider.hasPin() needs improvement #158

Closed
trim09 opened this Issue May 6, 2015 · 5 comments

Comments

Projects
None yet
2 participants
@trim09

trim09 commented May 6, 2015

I was able to toggle with a pin at ~600Hz on Raspberry Pi B+.

It is caused by expensive method RaspiGpioProvider.hasPin() that calls GpioUtil.isPinSupported() that calls isPinValid() that calls getEdgePin() that calls piBoardId() that opens file descriptors and is very slow.

I think we need different approach to check whether pin is available or not. Maybe the pin availability can be cached by some Hashmap or so.

Observed in 1.1-SNAPSHOT

@savageautomate savageautomate added the bug label May 6, 2015

@savageautomate savageautomate added this to the RELEASE 1.1 milestone May 6, 2015

@trim09

This comment has been minimized.

Show comment
Hide comment
@trim09

trim09 May 7, 2015

Thanks for adding it to Release 1.1. I am new to Pi4J so I will have a not very clever question. Is method RaspiGpioProvider.hasPin() static or dynamic? I mean for for example Raspberry pi has a pin X and for this pin it returns true. But does it return false when I start using the pin X for let say PWM?

Just FYI: I got ~180kHz when I overriden hasPin() that now always returns true for my testing.
And I got ~1MHz when I remover super.setState() call from the RaspiGpioProvider.setState(). I did it because this.getPinCache() in setState() method was the next bottleneck and it looked like the cache is never used for RaspiGpioProvider. But I agree that this is not a good solution because setState() calls dispatchPinDigitalStateChangeEvent(), Nevertheless it can be called directly from the RaspiGpioProvider.

Here is the code that I used for testing:
public class FastRaspiGpioProvider extends RaspiGpioProvider {
@override
public boolean hasPin(Pin pin) {
return true;
}

@Override
public void setState(Pin pin, PinState state) {
    super.dispatchPinDigitalStateChangeEvent(pin, state);
    Gpio.digitalWrite(pin.getAddress(), state.getValue());
}

}

Best regards,
Tomas

trim09 commented May 7, 2015

Thanks for adding it to Release 1.1. I am new to Pi4J so I will have a not very clever question. Is method RaspiGpioProvider.hasPin() static or dynamic? I mean for for example Raspberry pi has a pin X and for this pin it returns true. But does it return false when I start using the pin X for let say PWM?

Just FYI: I got ~180kHz when I overriden hasPin() that now always returns true for my testing.
And I got ~1MHz when I remover super.setState() call from the RaspiGpioProvider.setState(). I did it because this.getPinCache() in setState() method was the next bottleneck and it looked like the cache is never used for RaspiGpioProvider. But I agree that this is not a good solution because setState() calls dispatchPinDigitalStateChangeEvent(), Nevertheless it can be called directly from the RaspiGpioProvider.

Here is the code that I used for testing:
public class FastRaspiGpioProvider extends RaspiGpioProvider {
@override
public boolean hasPin(Pin pin) {
return true;
}

@Override
public void setState(Pin pin, PinState state) {
    super.dispatchPinDigitalStateChangeEvent(pin, state);
    Gpio.digitalWrite(pin.getAddress(), state.getValue());
}

}

Best regards,
Tomas

savageautomate added a commit that referenced this issue Sep 7, 2015

@savageautomate

This comment has been minimized.

Show comment
Hide comment
@savageautomate

savageautomate Sep 7, 2015

Member

Optimization now included in the 'develop' branch and in the Pi4J-1.1-SNAPSHOT builds.

On the Rpi2B ...

Before any changes. ~900Hz.
After optimizing the RaspiGpioProvider.hasPin() bottleneck ... ~260 kHz
After optimizing the super.setState() bottleneck ... ~700 kHz

This is about as good as I have been able to get it with the current implementation without removing any safety valves. (checks for pin support, mode type, etc).

For users needing more speed out of the GPIO's form Java, I suggest using the WiringPi wrappers included in Pi4J. These static functions are direct calls to the JNI layer where WiringPi performs the actual work to change the GPIO pin state. This eliminates the overhead from the Pi4J GPIO interfaces. But note these direct JNI methods are far less forgiving if you make a mistake and may result in a program crash.

see: https://github.com/Pi4J/pi4j/blob/master/pi4j-core/src/main/java/com/pi4j/wiringpi/Gpio.java#L348

WiringPi example code: https://github.com/Pi4J/pi4j/blob/master/pi4j-example/src/main/java/WiringPiGpioExample.java

Thanks, Robert

Member

savageautomate commented Sep 7, 2015

Optimization now included in the 'develop' branch and in the Pi4J-1.1-SNAPSHOT builds.

On the Rpi2B ...

Before any changes. ~900Hz.
After optimizing the RaspiGpioProvider.hasPin() bottleneck ... ~260 kHz
After optimizing the super.setState() bottleneck ... ~700 kHz

This is about as good as I have been able to get it with the current implementation without removing any safety valves. (checks for pin support, mode type, etc).

For users needing more speed out of the GPIO's form Java, I suggest using the WiringPi wrappers included in Pi4J. These static functions are direct calls to the JNI layer where WiringPi performs the actual work to change the GPIO pin state. This eliminates the overhead from the Pi4J GPIO interfaces. But note these direct JNI methods are far less forgiving if you make a mistake and may result in a program crash.

see: https://github.com/Pi4J/pi4j/blob/master/pi4j-core/src/main/java/com/pi4j/wiringpi/Gpio.java#L348

WiringPi example code: https://github.com/Pi4J/pi4j/blob/master/pi4j-example/src/main/java/WiringPiGpioExample.java

Thanks, Robert

@trim09

This comment has been minimized.

Show comment
Hide comment
@trim09

trim09 Sep 8, 2015

Perfect! ~700 kHz is good enough for many applications - including mine. Thanks Robert

Tomas

trim09 commented Sep 8, 2015

Perfect! ~700 kHz is good enough for many applications - including mine. Thanks Robert

Tomas

@trim09

This comment has been minimized.

Show comment
Hide comment
@trim09

trim09 Sep 8, 2015

and if not - WiringPi is there :-)

trim09 commented Sep 8, 2015

and if not - WiringPi is there :-)

@savageautomate

This comment has been minimized.

Show comment
Hide comment
@savageautomate

savageautomate Sep 8, 2015

Member

... Is method RaspiGpioProvider.hasPin() static or dynamic? I mean for for example Raspberry pi has a pin X and for this pin it returns true. But does it return false when I start using the pin X for let say PWM?

The hasPin() was originally put in place to ensure that a user did not attempt to provision or use a Pin that a given provider (such as the RaspiGpioProvider) did not support. For example, if you look in the Pi4J GPIO Extensions project you will see other GpioProviders for DACs, ADCs, and GPIO expander chips. So this was really just a safeguard to make sure the specific GpioProvider instance was the correct GpioProvider for the given Pin instance. Without this, bad code could cause unpredictable results :-)

Later on, I added the isPinSupported() in the RasiGpioProvider because with the addition of the Compute Module and A+, B+ and 2B models, new pins could exist that did not exist on earlier models. The inadvertent side-effect was this dramatic decrease in GPIO toggling performance.

Thanks, Robert

Member

savageautomate commented Sep 8, 2015

... Is method RaspiGpioProvider.hasPin() static or dynamic? I mean for for example Raspberry pi has a pin X and for this pin it returns true. But does it return false when I start using the pin X for let say PWM?

The hasPin() was originally put in place to ensure that a user did not attempt to provision or use a Pin that a given provider (such as the RaspiGpioProvider) did not support. For example, if you look in the Pi4J GPIO Extensions project you will see other GpioProviders for DACs, ADCs, and GPIO expander chips. So this was really just a safeguard to make sure the specific GpioProvider instance was the correct GpioProvider for the given Pin instance. Without this, bad code could cause unpredictable results :-)

Later on, I added the isPinSupported() in the RasiGpioProvider because with the addition of the Compute Module and A+, B+ and 2B models, new pins could exist that did not exist on earlier models. The inadvertent side-effect was this dramatic decrease in GPIO toggling performance.

Thanks, Robert

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment