-
Notifications
You must be signed in to change notification settings - Fork 47
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
wait time as a span #19
Conversation
Signed-off-by: zhongzc <zhongzc_arch@outlook.com>
Signed-off-by: zhongzc <zhongzc_arch@outlook.com>
Signed-off-by: zhongzc <zhongzc_arch@outlook.com>
Signed-off-by: zhongzc <zhongzc_arch@outlook.com>
Signed-off-by: zhongzc <zhongzc_arch@outlook.com>
Signed-off-by: zhongzc <zhongzc_arch@outlook.com>
Signed-off-by: zhongzc <zhongzc_arch@outlook.com>
Signed-off-by: zhongzc <zhongzc_arch@outlook.com>
Signed-off-by: zhongzc <zhongzc_arch@outlook.com>
Signed-off-by: zhongzc <zhongzc_arch@outlook.com>
@@ -46,6 +88,6 @@ impl Drop for Collector { | |||
fn drop(&mut self) { | |||
self.inner | |||
.closed | |||
.store(true, std::sync::atomic::Ordering::SeqCst); | |||
.store(true, std::sync::atomic::Ordering::Relaxed); |
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.
Any explanation about why Relaxed
is enough now?
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.
Ordering is about two or more shared variables among multiple threads. If we don't have that synchronization semantics, relaxed is enough.
@@ -42,14 +42,12 @@ static GLOBAL_ID_COUNTER: std::sync::atomic::AtomicU32 = std::sync::atomic::Atom | |||
|
|||
#[inline] | |||
fn next_global_id_prefix() -> u32 { | |||
GLOBAL_ID_COUNTER.fetch_add(1, std::sync::atomic::Ordering::SeqCst) | |||
GLOBAL_ID_COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed) |
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.
Ditto
// fill the elapsed cycles of the first span | ||
tl.spans[self.span_start_index].elapsed_cycles = | ||
minstant::now().saturating_sub(tl.spans[self.span_start_index].begin_cycles); | ||
// fill the elapsed cycles of the first span (except the leading span) |
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.
Any explanation about why created a new concept LeadingSpan
and change logic here?
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.
A leading span is one of Root
, Spawning
, and Scheduling
, which represents how a tracing context creates.
The elapsed time of a leading span means how long it waits from creating the task to starting execution.
A Settle
span means the execution is starting.
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.
A leading span is one of
Root
,Spawning
, andScheduling
, which represents how a tracing context creates.The elapsed time of a leading span means how long it waits from creating the task to starting execution.
A
Settle
span means the execution is starting.
Maybe this response can add to the comments in L142
Signed-off-by: zhongzc <zhongzc_arch@outlook.com>
Signed-off-by: zhongzc zhongzc_arch@outlook.com
Changes:
span_sets
inTraceDetails
:State
between spans:State::Root
,State::Settle
,State::Local
,State::Spawning
,State::Scheduling
.Root
,Spawning
andScheduling
are spans which indicate waiting for execution.