Skip to content
This repository has been archived by the owner on May 17, 2021. It is now read-only.

[plugwise] Fix #1003: high CPU load #4162

Merged
merged 2 commits into from
Jun 22, 2016

Conversation

jowiho
Copy link
Contributor

@jowiho jowiho commented Mar 12, 2016

Fixes #1003

Fixed high CPU load of plugswise binding, by removing a busy
loop in the sendMessage code.
Simplified sending and proecssing of messages by replacing misused
Quartz jobs with straightforward background threads. Clean up
locking of the various queues in the Stick.

Fixed high CPU load of plugswise binding, by removing a busy
loop in the sendMessage code.
Simplified sending and proecssing of messages by replacing misused
Quartz jobs with straightforward background threads. Clean up
locking of the various queues in the Stick.
@teichsta
Copy link
Member

Thanks @jowiho for this contribution. Can somebody else (except you) confirm this fix works? Others created discussion threads at community.openhab.org in order to find Testers …

@teichsta teichsta added this to the 1.9.0 milestone Mar 13, 2016
@jowiho
Copy link
Contributor Author

jowiho commented Mar 13, 2016

Hi @teichsta, I've posted a request for testing in the original issue thread. Let's see if anyone responds.

@teichsta
Copy link
Member

great, thanks!

@watou
Copy link
Contributor

watou commented May 2, 2016

@jowiho Has anyone responded on the forum, or could you ask for testers again? Thanks for this.

@jowiho
Copy link
Contributor Author

jowiho commented May 2, 2016

@watou There's one reply on the forum (#1003) that confirms that my patch does what it is supposed to do.

@watou
Copy link
Contributor

watou commented May 2, 2016

@jowiho Thanks, did you make a discussion topic on the forum at https://community.openhab.org/ ? Many more potential plugwise users are likely to read https://community.openhab.org/ than the issues list. It's encouraging that one user did report success. Do you think this needs further testing? In any case, could you resolve the conflicts to prepare it for merge? Many thanks!

@teichsta
Copy link
Member

teichsta commented May 6, 2016

In any case, could you resolve the conflicts to prepare it for merge?

@jowiho could you please?

@jowiho
Copy link
Contributor Author

jowiho commented May 9, 2016

I'll fix the conflict as soon as I have time (within a few days).

@jowiho
Copy link
Contributor Author

jowiho commented May 17, 2016

@teichsta I've resolved the merge conflicts. The PR is ready for merge again.

@jhoekstra
Copy link

Is there a way to test this binding with a repo based install? Got very high CPU-usage as well using plugwise.

@watou
Copy link
Contributor

watou commented May 22, 2016

@jhoekstra I just made a JAR from the current state of this PR, where you could replace the current JAR with it. Please report back here if it appears to work as advertised, or if it doesn't.

Before merging this PR, please verify that it's properly rebased to master, because I pulled the PR locally to build the JAR, and the list of changed files I see in SourceTree seems incorrect.

@jhoekstra
Copy link

With the binding from Watou I see a considerable drop in load.
CPU usage went down from 75-80% to 10-15% on a dualcore VM.
So works as advertised :)

@watou
Copy link
Contributor

watou commented May 22, 2016

Great news! Now to verify that the PR files changed are correct before merge, as mentioned above. Thanks for testing @jhoekstra!

@teichsta
Copy link
Member

since the PR shows one changed files nothing else seem to be changed … i certainly didn't get your point @watou

@watou
Copy link
Contributor

watou commented May 31, 2016

The PR branch looked like it had other commits in it, but it seems OK on reinspection, so please proceed without any delay on my account.

@watou watou merged commit 2164577 into openhab:master Jun 22, 2016
@watou
Copy link
Contributor

watou commented Jun 22, 2016

Thanks @jowiho! Could you and @jhoekstra test the Cloudbees build build # 1259 or later (after this merge; build not ready as of this writing)? Just to double check all is well?

@jhoekstra
Copy link

Seems ok to me, low load and switching like before.

@watou
Copy link
Contributor

watou commented Jun 23, 2016

Great to hear that, thanks @jhoekstra!

@jowiho jowiho deleted the issue-1003-fix-high-cpu-load branch August 10, 2016 20:44
cdjackson added a commit to cdjackson/openhab1-addons that referenced this pull request Aug 12, 2016
* master: (178 commits)
  Fix the generateDeviceId method. (openhab#4498)
  [modbus] Add support for int32_swap, uint32_swap and float32_swap data types (openhab#4251)
  Extend error logging on retry failure (openhab#4477)
  Changed timestamp of item insert from MySql server time to local Java… (openhab#4503)
  [mochadx10] Add binding to OH2 distro (openhab#4502)
  [mqtt] Add openhab-action-mqtt feature for OH2 install (openhab#4499)
  ZWave database update (openhab#4497)
  Fixes openhab#1003 (openhab#4162)
  [WeatherBinding] Add log messages for invalid configurations (openhab#4458)
  Network Health - Made log message more clear (openhab#4425)
  Update ZMNHSD (openhab#4490)
  Update clock command class and converter (openhab#4489)
  Updated modbus config template with serial example. Documented advanced connection parameters (openhab#4487)
  ZWave update WA105DBZ (openhab#4485)
  Zwave database 180616 (openhab#4483)
  Zwave database 180616 (openhab#4479)
  Add Aeon ZW112 - Door/Window Sensor 6 (openhab#4472)
  Include Aeon ZW112 - Door/Window Sensor 6 (openhab#4471)
  Z-Wave: Added support for CENTRAL_SCENE (openhab#4431)
  ZWave database update (openhab#4476)
  ...

# Conflicts:
#	bundles/binding/org.openhab.binding.zwave/database/products.xml
#	bundles/binding/org.openhab.binding.zwave/src/main/java/org/openhab/binding/zwave/internal/converter/ZWaveConverterHandler.java
#	bundles/binding/org.openhab.binding.zwave/src/main/java/org/openhab/binding/zwave/internal/protocol/commandclass/ZWaveCommandClass.java
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants