Skip to content
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.

Sign up for GitHub

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

Jump to bottom

console: bold field names #35

Merged
merged 5 commits into from
Jun 14, 2021
Merged

console: bold field names #35

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
86d1a37
bold field names
zaharidichev May 27, 2021
f1dc5bc
Merge branch 'main' into zd/fields-on-task-struct
hawkw May 27, 2021
81771ad
feedback
zaharidichev May 31, 2021
c4e863a
Apply suggestions from code review
hawkw Jun 14, 2021
901d60e
Merge branch 'main' into zd/fields-on-task-struct
hawkw Jun 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 111 additions & 15 deletions console/src/tasks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,21 @@ use std::{
cell::RefCell,
collections::HashMap,
convert::TryFrom,
fmt::Write,
fmt,
rc::{Rc, Weak},
sync::Arc,
time::{Duration, SystemTime},
};
use tui::{
layout,
style::{self, Style},
text,
style::{self, Modifier, Style},
text::{self, Span, Spans},
widgets::{Block, Cell, Row, Table, TableState},
};
#[derive(Debug)]
pub(crate) struct State {
tasks: HashMap<u64, Rc<RefCell<Task>>>,
metas: HashMap<u64, Metadata>,
sorted_tasks: Vec<Weak<RefCell<Task>>>,
sort_by: SortBy,
table_state: TableState,
Expand All @@ -39,12 +41,19 @@ enum SortBy {
pub(crate) struct Task {
id: u64,
id_hex: String,
fields: String,
fields: Vec<Field>,
formatted_fields: Vec<Span<'static>>,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is that 'static correct.. 😕

kind: &'static str,
stats: Stats,
completed_for: usize,
}

#[derive(Debug)]
pub(crate) struct Metadata {
field_names: Vec<Arc<str>>,
//TODO: add more metadata as needed
}

#[derive(Debug)]
struct Stats {
polls: u64,
Expand All @@ -54,6 +63,21 @@ struct Stats {
total: Option<Duration>,
}

#[derive(Debug)]
pub(crate) struct Field {
pub(crate) name: Arc<str>,
pub(crate) value: FieldValue,
}

#[derive(Debug)]
pub(crate) enum FieldValue {
Bool(bool),
Str(String),
U64(u64),
I64(i64),
Debug(String),
}

impl State {
// How many updates to retain completed tasks for
const RETAIN_COMPLETED_FOR: usize = 6;
Expand Down Expand Up @@ -114,31 +138,67 @@ impl State {
if let Some(now) = update.now {
self.last_updated_at = Some(now.into());
}

if let Some(new_metadata) = update.new_metadata {
let metas = new_metadata.metadata.into_iter().map(|meta| {
let id = meta.id.expect("no id").id;
let metadata = meta.metadata.expect("no metadata");
(id, metadata.into())
});
hawkw marked this conversation as resolved.
Show resolved Hide resolved
self.metas.extend(metas);
}

let mut stats_update = update.stats_update;
let sorted = &mut self.sorted_tasks;
let new_tasks = update.new_tasks.into_iter().filter_map(|task| {
let metas = &mut self.metas;
let new_tasks = update.new_tasks.into_iter().filter_map(|mut task| {
if task.id.is_none() {
tracing::warn!(?task, "skipping task with no id");
}
let kind = match task.kind() {
proto::tasks::task::Kind::Spawn => "T",
proto::tasks::task::Kind::Blocking => "B",
};
let fields = task

let fields: Vec<Field> = task
.fields
.iter()
.fold(String::new(), |mut res, f| {
write!(&mut res, "{} ", f).unwrap();
res
.drain(..)
.filter_map(|f| {
let field_name = f.name.as_ref().expect("no name");
hawkw marked this conversation as resolved.
Show resolved Hide resolved
let name: Option<Arc<str>> = match field_name {
proto::field::Name::StrName(n) => Some(n.clone().into()),
proto::field::Name::NameIdx(idx) => {
let meta_id = f.metadata_id.as_ref().expect("no id");
hawkw marked this conversation as resolved.
Show resolved Hide resolved
metas
.get(&meta_id.id)
.and_then(|meta| meta.field_names.get(*idx as usize))
.cloned()
Comment on lines +172 to +175
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it might be worth representing the metadata's field names as Arc<str>s, so that they are cheap to clone --- once the subscriber populates the metadata field names, we can expect most spans to have field names stored as indices rather than strings, so we could significantly reduce the number of strings the console allocates if we only stored a single copy of those strings in the metadata, and cloned an Arc into the task's fields.

it would be fine to do this in a follow-up PR, though --- this is fine for now.

}
};
let value = f.value.as_ref().expect("no value").clone().into();
name.map(|name| Field { name, value })
})
.trim_end()
.into();
.collect();

let formatted_fields = fields.iter().fold(Vec::default(), |mut acc, f| {
acc.extend(vec![
Span::styled(
f.name.to_string(),
Style::default().add_modifier(Modifier::BOLD),
),
Span::from("="),
Span::from(format!("{} ", f.value)),
]);
acc
});

let id = task.id?.id;
let stats = stats_update.remove(&id)?.into();
let mut task = Task {
id,
id_hex: format!("{:x}", id),
fields,
formatted_fields,
kind,
stats,
completed_for: 0,
Expand Down Expand Up @@ -182,6 +242,7 @@ impl State {
let rows = self.sorted_tasks.iter().filter_map(|task| {
let task = task.upgrade()?;
let task = task.borrow();

let mut row = Row::new(vec![
Cell::from(task.id_hex.to_string()),
// TODO(eliza): is there a way to write a `fmt::Debug` impl
Expand All @@ -206,7 +267,7 @@ impl State {
prec = DUR_PRECISION,
)),
Cell::from(format!("{:>width$}", task.stats.polls, width = POLLS_LEN)),
Cell::from(task.fields.to_string()),
Cell::from(Spans::from(task.formatted_fields.clone())),
]);
if task.completed_for > 0 {
row = row.style(Style::default().add_modifier(style::Modifier::DIM));
Expand Down Expand Up @@ -330,6 +391,7 @@ impl Default for State {
fn default() -> Self {
Self {
tasks: Default::default(),
metas: Default::default(),
sorted_tasks: Default::default(),
sort_by: Default::default(),
selected_column: SortBy::default() as usize,
Expand All @@ -345,8 +407,8 @@ impl Task {
&self.id_hex
}

pub(crate) fn fields(&self) -> &str {
&self.fields
pub(crate) fn formatted_fields(&self) -> &Vec<Span<'static>> {
&self.formatted_fields
}

pub(crate) fn total(&self, since: SystemTime) -> Duration {
Expand Down Expand Up @@ -397,6 +459,26 @@ impl From<proto::tasks::Stats> for Stats {
}
}

impl From<proto::Metadata> for Metadata {
fn from(pb: proto::Metadata) -> Self {
Self {
field_names: pb.field_names.into_iter().map(|n| n.into()).collect(),
}
}
}

impl From<proto::field::Value> for FieldValue {
fn from(pb: proto::field::Value) -> Self {
match pb {
proto::field::Value::BoolVal(v) => Self::Bool(v),
proto::field::Value::StrVal(v) => Self::Str(v),
proto::field::Value::I64Val(v) => Self::I64(v),
proto::field::Value::U64Val(v) => Self::U64(v),
proto::field::Value::DebugVal(v) => Self::Debug(v),
}
}
}

impl Default for SortBy {
fn default() -> Self {
Self::Total
Expand Down Expand Up @@ -437,3 +519,17 @@ impl TryFrom<usize> for SortBy {
}
}
}

impl fmt::Display for FieldValue {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
FieldValue::Bool(v) => fmt::Display::fmt(v, f)?,
FieldValue::Str(v) => fmt::Display::fmt(v, f)?,
FieldValue::U64(v) => fmt::Display::fmt(v, f)?,
FieldValue::Debug(v) => fmt::Display::fmt(v, f)?,
FieldValue::I64(v) => fmt::Display::fmt(v, f)?,
}

Ok(())
}
}
10 changes: 6 additions & 4 deletions console/src/view/task.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,18 @@ impl TaskView {
let task = &*self.task.borrow();
const DUR_PRECISION: usize = 4;

let attrs = Spans::from(vec![
let mut attrs = vec![
Span::styled("ID: ", Style::default().add_modifier(style::Modifier::BOLD)),
Span::raw(task.id_hex()),
Span::raw(", "),
Span::styled(
"Fields: ",
Style::default().add_modifier(style::Modifier::BOLD),
),
Span::raw(task.fields()),
]);
];

attrs.extend(task.formatted_fields().clone());
let atributes = Spans::from(attrs);

let metrics = Spans::from(vec![
Span::styled(
Expand All @@ -66,7 +68,7 @@ impl TaskView {
Span::from(format!("{:.prec$?}", task.idle(now), prec = DUR_PRECISION,)),
]);

let lines = vec![attrs, metrics];
let lines = vec![atributes, metrics];
let block = Block::default().borders(Borders::ALL).title("Task");
let paragraph = Paragraph::new(lines).block(block);
frame.render_widget(paragraph, area);
Expand Down