-
Notifications
You must be signed in to change notification settings - Fork 58
Reduce busy-waiting CPU load in waitEvents() #201
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | |||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -41,6 +41,7 @@ public class SerialPort { | ||||||||||||||||||||||||||
| private final String portName; | |||||||||||||||||||||||||||
| private volatile boolean portOpened = false; | |||||||||||||||||||||||||||
| private boolean maskAssigned = false; | |||||||||||||||||||||||||||
| private int waitEventsTimeoutMs = -1; | |||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||
| //since 2.2.0 -> | |||||||||||||||||||||||||||
| private volatile Method methodErrorOccurred = null; | |||||||||||||||||||||||||||
|
|
@@ -920,6 +921,34 @@ public boolean setFlowControlMode(int mask) throws SerialPortException { | ||||||||||||||||||||||||||
| return serialInterface.setFlowControlMode(portHandle, mask); | |||||||||||||||||||||||||||
| } | |||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||
| /** | |||||||||||||||||||||||||||
| * Reduce busy-waiting CPU load in {@link #waitEvents()}. | |||||||||||||||||||||||||||
| * | |||||||||||||||||||||||||||
| * (This is irrelevant for windows) | |||||||||||||||||||||||||||
| * | |||||||||||||||||||||||||||
| * The {@link #waitEvents()} implementation on non-windows systems | |||||||||||||||||||||||||||
| * usually returns immediately. This is unfortunate for the callers | |||||||||||||||||||||||||||
| * which want to await events in an infinite-loop. As doing so would | |||||||||||||||||||||||||||
| * burn lot of CPU time. | |||||||||||||||||||||||||||
| * | |||||||||||||||||||||||||||
| * This setting can be used to reduce that load. For regular | |||||||||||||||||||||||||||
| * incoming data events this does not cause any further delays. | |||||||||||||||||||||||||||
| * {@link #waitEvents()} still will reports most of the events as | |||||||||||||||||||||||||||
| * soon they become available, even before the specified timeout got | |||||||||||||||||||||||||||
| * reached. | |||||||||||||||||||||||||||
| * | |||||||||||||||||||||||||||
| * BUT BE AWARE: Enabling this might delay delivery of some special | |||||||||||||||||||||||||||
| * serial-events (like 'DCD line changed' or 'RI line changed') by | |||||||||||||||||||||||||||
| * the amount of time specified. So you have to decide yourself if | |||||||||||||||||||||||||||
| * you can/will afford this trade. | |||||||||||||||||||||||||||
| */ | |||||||||||||||||||||||||||
| public void setWaitEventsTimeoutMs(int waitEventsTimeoutMs) { | |||||||||||||||||||||||||||
| if (waitEventsTimeoutMs <= 0) { | |||||||||||||||||||||||||||
| throw new IllegalArgumentException(String.valueOf(waitEventsTimeoutMs)); | |||||||||||||||||||||||||||
| } | |||||||||||||||||||||||||||
| this.waitEventsTimeoutMs = waitEventsTimeoutMs; | |||||||||||||||||||||||||||
| } | |||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||
| /** | |||||||||||||||||||||||||||
| * Get flow control mode | |||||||||||||||||||||||||||
| * | |||||||||||||||||||||||||||
|
|
@@ -951,7 +980,7 @@ public boolean sendBreak(int duration)throws SerialPortException { | ||||||||||||||||||||||||||
| } | |||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||
| private int[][] waitEvents() { | |||||||||||||||||||||||||||
| return serialInterface.waitEvents(portHandle); | |||||||||||||||||||||||||||
| return serialInterface.waitEvents2(portHandle, waitEventsTimeoutMs); | |||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please explain the rationale behind keeping the old API intact? We don't really promise the ability to swap native binaries and from what I can tell, the old
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TL:DR; Not strictly necessary, just preferrable for the project where I have to use jssc 🙈
This is the closest fit 🙂 Unluckily the project I'm working on (which uses JSSC) has an overly complicated setup. This project, not intentionally but more indirectly seems to depends on deploying the binary through another way than the java parts (agree, ugly). Due to way too complicated reasons. There's a way too long chain of technical details why this is the case 😔 So yes, the java part in unlucky circumstances can be a mismatching version compared to the shared objects 🙁
I'll throw that question in my team and we'll see what happens 🙂 Maybe we come up with an easlier migration strategy. Edit: Hmm. I think I should verify again, if that "separate-distribution" thing really is true 🤔 Need to have a look with co-workers. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since the code change in this PR is essentially native-only, I feel like one could objective just reduce the entropy by half by simply not touching the JAR at all but I'm probably misunderstanding something. I totally get the "tech-debt" stuff... I've moved all of my systems away from letting the JAR deploy the native lib per #92 so I'm 100% with you there, I just don't particularly care for namespacing obsolete code that's never to be used again. I'm probably misunderstanding something but I feel like you can:
... assuming your versioning of the two must be independent. Sorry if I'm missing something or minimizing your struggles. The project is very grateful to have you help with the C++ portions, so we can certainly tackle this as you wish and add
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We had another look onto our situation. Looks like parts of my assumption were wrong. This means, we should be safe to get rid of the backwards compatibility stuff 🥳 I'll give it a try 👍 |
|||||||||||||||||||||||||||
| } | |||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||
| /** | |||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIR, we produce production binaries with dockcross in
workflows/cross-compile.yml, so this can probably safely be reverted. Reference: #190There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh 🙂 yes. Looks like I was missing that.