Skip to content
This repository has been archived by the owner. It is now read-only.

Optimize CPU Monitor: 25% perf boosts and other changes #460

Merged
merged 11 commits into from Jun 23, 2020

Conversation

psibi
Copy link
Contributor

@psibi psibi commented Jun 14, 2020

Summary of the changes:

  • Make the CPU monitor more type safe. Added various new types instead of having [String] and [Float] everywhere.
  • Add test cases for the CPU monitor
  • Introduce new type named PureConfig. This is a pure version of MConfig and the initial work has been done in this MR which will hopefully in the future remove the need of the whole MConfig type.
  • Avoid template parsing for the components which won't be rendered
  • Avoid formatting output for the components which won't be rendered
  • Perf improvements:

Before:

benchmarked Cpu Benchmarks/CPU normal args
time                 101.6 μs   (100.8 μs .. 102.3 μs)
                     0.999 R²   (0.997 R² .. 1.000 R²)
mean                 103.0 μs   (102.4 μs .. 104.5 μs)
std dev              2.883 μs   (1.465 μs .. 5.154 μs)
variance introduced by outliers: 11% (moderately inflated)

After:

time                 75.80 μs   (75.15 μs .. 76.36 μs)
                     1.000 R²   (0.999 R² .. 1.000 R²)
mean                 75.97 μs   (75.74 μs .. 76.41 μs)
std dev              1.057 μs   (666.6 ns .. 1.927 μs)

@jaor
Copy link
Owner

jaor commented Jun 14, 2020

This looks very good indeed. I am very happy to see tests too. I have just a nit, that might be a bit annoying, but here it goes: i don't like the PureConfig name, because 'pure' just describes an implementation detail, which is neither here nor there (it feels like "typed Hungarian notation" if you take my meaning). How about we call it simply MonitorConfig?

@psibi
Copy link
Contributor Author

psibi commented Jun 14, 2020

@jaor Thanks for the feedback! I will do the changes tomorrow. Free feel to let me know other changes too (including naming!). :-)

@aniketd
Copy link

aniketd commented Jun 15, 2020

Awesome! Thanks @psibi !

src/Xmobar/Plugins/Monitors/Common/Output.hs Show resolved Hide resolved
src/Xmobar/Plugins/Monitors/Common/Output.hs Outdated Show resolved Hide resolved
src/Xmobar/Plugins/Monitors/Common/Output.hs Outdated Show resolved Hide resolved
src/Xmobar/Plugins/Monitors/Common/Output.hs Show resolved Hide resolved
@psibi
Copy link
Contributor Author

psibi commented Jun 20, 2020

@jaor / @slotThe I have updated the PR to address your comments.

jaor
jaor approved these changes Jun 23, 2020
@jaor jaor merged commit 23171ad into jaor:master Jun 23, 2020
1 check passed
@jaor
Copy link
Owner

jaor commented Jun 23, 2020

merged, thanks! (even travis-ci is working now!)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants