Follow new naming in GHC.Stats, add new stats

haskell-github-trust / ekg-core

Library for tracking system metrics

BSD 3-Clause "New" or "Revised" License

40 stars 39 forks source link

Follow new naming in GHC.Stats, add new stats #30

Open nh2 opened 5 years ago

nh2 commented 5 years ago

Fixes #29.

Based on #28 for GHC 8.6 compatibility.

I've tested it with:

Stackage lts-12.17
Stackage nightly-2018-11-08

nh2 commented 5 years ago

Urgh, cumulative_par_balanced_copied_bytes has a missing @since 4.12.0.0 in https://downloads.haskell.org/~ghc/master/libraries/html/base/GHC-Stats.html, will fix.

nh2 commented 5 years ago

Looks like the ekg package's Javascript also has to be updated, it refers to the old names and there's no compile-time check for that:

https://github.com/tibbe/ekg/blob/07ac776ef90b8d6f5629568a5a28d911a0e267b0/assets/monitor.js#L337

That's why all my graphs are empty if just applying this ekg-core patch, and all values in the top right are 0.

23Skidoo commented 5 years ago

I think I'll do a GHC 8.6-compatible point release first, and then a major release with this change. ekg-statsd, ekg-json, and ekg may also require updates.

nh2 commented 5 years ago

I think I'll do a GHC 8.6-compatible point release first, and then a major release with this change.

That sounds prudent to me.

ekg-statsd, ekg-json, and ekg may also require updates.

ekg for sure, I already have a branch that does this which I'm currently testing. The other two I'm not using so I might need some help with those.

23Skidoo commented 5 years ago

I think I'll do a GHC 8.6-compatible point release first

This is now done.

nh2 commented 4 years ago

Doesn't seem like a great idea to me. Can't we use new names with base <4.10 as well?

I have force-pushed to implement this suggestion (and also to address all other feedback). Now it translates the old API to the new field names.

For easy review comparison I have diffed the fields used by not(MIN_VERSION_base(4,10,0)) and MIN_VERSION_base(4,10,0) in meld, showing that the first is a subset of the other:

The change has now been tested on Stackage versions:

lts-14.27 (GHC 8.6.5)
nightly-2018-11-08 (GHC 8.6.1)
lts-12.17 (GHC 8.4.4)
lts-9.21 (ghc-8.0.2) -- this is base 4.9

nh2 commented 4 years ago

ekg javascript/HTML PR at https://github.com/tibbe/ekg/pull/80

nh2 commented 4 years ago

ekg-statsd, ekg-json, and ekg may also require updates.

ekg: https://github.com/tibbe/ekg/pull/80 (only needs a few fields renamed)
ekg-json: seems to require no update, as it does not refer to any specific field names
ekg-statsd: same

eborden commented 2 years ago

This PR seems worth resurrecting. There are now additional stats from base-4.14.1.0+ tracking the non moving GC. What is pending to get this across the line. How can I help?

eborden commented 2 years ago

The newer stats are

Stats.RTSStats
  { nonmoving_gc_sync_cpu_ns
  , nonmoving_gc_sync_elapsed_ns
  , nonmoving_gc_sync_max_elapsed_ns
  , nonmoving_gc_cpu_ns
  , nonmoving_gc_elapsed_ns
  , nonmoving_gc_max_elapsed_ns
  }

and

Stats.GCDetails
  { gcdetails_nonmoving_gc_sync_cpu_ns
  , gcdetails_nonmoving_gc_sync_elapsed_ns
  }

NorfairKing commented 1 year ago

Gentle ping, another year later