counters #287
counters #287
Conversation
src/counter.rs
Outdated
| pub lograte: usize, | ||
| } | ||
|
|
||
| macro_rules! STATIC_COUNTER { |
There was a problem hiding this comment.
Can you make lowercase? And is there a reason for it to include the word static?
There was a problem hiding this comment.
because its for static initialization of the counter. its kind of a special thing, we are instrumenting the code with a global
src/counter.rs
Outdated
| use std::time::{Duration, UNIX_EPOCH, SystemTime}; | ||
|
|
||
| pub struct Counter { | ||
| pub name: &'static str, |
There was a problem hiding this comment.
we are using a static, I think str is more appropriate.
src/counter.rs
Outdated
|
|
||
| macro_rules! STATIC_COUNTER_INC { | ||
| ($name:expr, $count:expr, $start:expr) => { | ||
| unsafe { $name.inc($count, $start.elapsed()) }; |
There was a problem hiding this comment.
because its a mutable global, rust treats these as unsafe
| let nanos = self.nanos.fetch_add(total as usize, Ordering::Relaxed); | ||
| let times = self.times.fetch_add(1, Ordering::Relaxed); | ||
| if times % self.lograte == 0 && times > 0 { | ||
| let now = SystemTime::now().duration_since(UNIX_EPOCH).unwrap(); |
There was a problem hiding this comment.
Can you use Instant here, so that it's consistent with other uses of now() within the codebase?
There was a problem hiding this comment.
what would the instant be referencing as epoch? each counter needs to print its value against a global point of reference of time.
There was a problem hiding this comment.
can you use timing::timestamp() does what this does
Codecov Report
@@ Coverage Diff @@
## master #287 +/- ##
==========================================
+ Coverage 90.92% 91.07% +0.14%
==========================================
Files 32 33 +1
Lines 3284 3315 +31
==========================================
+ Hits 2986 3019 +33
+ Misses 298 296 -2
Continue to review full report at Codecov.
|
| let now = SystemTime::now().duration_since(UNIX_EPOCH).unwrap(); | ||
| let now_ms = now.as_secs() * 1_000 + now.subsec_nanos() as u64 / 1_000_000; | ||
| info!( | ||
| "COUNTER:{{\"name:\":\"{}\", \"counts\": {}, \"nanos\": {}, \"samples\": {} \"rate\": {}, \"now\": {}}}", |
There was a problem hiding this comment.
@sakridge should print out to a json formated blob
There was a problem hiding this comment.
Yes, I have a change that does that.
src/counter.rs
Outdated
| @@ -9,7 +9,7 @@ pub struct Counter { | |||
| pub lograte: usize, | |||
| } | |||
|
|
|||
| macro_rules! STATIC_COUNTER { | |||
| macro_rules! counter_ctor { | |||
There was a problem hiding this comment.
How about create_counter and inc_counter? <verb>_<subject>
|
Needs a rebase |
| let nanos = self.nanos.fetch_add(total as usize, Ordering::Relaxed); | ||
| let times = self.times.fetch_add(1, Ordering::Relaxed); | ||
| if times % self.lograte == 0 && times > 0 { | ||
| let now = SystemTime::now().duration_since(UNIX_EPOCH).unwrap(); |
There was a problem hiding this comment.
can you use timing::timestamp() does what this does
| let now = SystemTime::now().duration_since(UNIX_EPOCH).unwrap(); | ||
| let now_ms = now.as_secs() * 1_000 + now.subsec_nanos() as u64 / 1_000_000; | ||
| info!( | ||
| "COUNTER:{{\"name:\":\"{}\", \"counts\": {}, \"nanos\": {}, \"samples\": {} \"rate\": {}, \"now\": {}}}", |
There was a problem hiding this comment.
Yes, I have a change that does that.
Bumps [eslint](https://github.com/eslint/eslint) from 7.6.0 to 7.7.0. - [Release notes](https://github.com/eslint/eslint/releases) - [Changelog](https://github.com/eslint/eslint/blob/master/CHANGELOG.md) - [Commits](eslint/eslint@v7.6.0...v7.7.0) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…(backport of solana-labs#245) (solana-labs#287) [anza migration] fix: use the correct log filter for non-unix (solana-labs#245) fix: use the correct log filter for non-unix (cherry picked from commit 2537e3e) Co-authored-by: Yihau Chen <a122092487@gmail.com>
No description provided.