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
Add code for off-line reconstruction of candidate trees #212
Conversation
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.
Why create an offline_analysis
module containing a single tree
module? I'd just put this in telamon::explorer::offline_analysis
for now (this should be in explorer
since it is the explorer log).
src/offline_analysis/tree.rs
Outdated
/// becomes the deadend time if no other thread has declared the | ||
/// node as a deadend with a lower timestamp. | ||
pub fn declare_deadend(&mut self, timestamp: Duration) { | ||
if !self.is_deadend() || self.deadend_time.unwrap() > timestamp { |
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.
You typically want to avoid the "check if some and then unwrap()" pattern and use a single check instead; e.g.
pub fn declare_deadend(&mut self, mut timestamp: Duration) {
if let Some(old_timestamp) = self.deadend_time {
timestamp = cmp::min(timestamp, old_timestamp)
}
self.deadend_time = Some(timestamp);
}
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.
It looks a bit weird that the update then becomes unconditional.
BTW it would be good to provide an API for creating a |
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.
Nothing to add on the general idea, but a few Rust/style related changes. In particular, Rust tends to let the caller do the clone on the returned value rather than the callee.
src/offline_analysis/tree.rs
Outdated
///! candidate tree from a log file | ||
use crate::explorer::choice::ActionEx as Action; | ||
use std::cell::RefCell; | ||
use std::collections::HashMap; |
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.
Use utils::Hashmap
instead as this one is slow and non-deterministic.
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.
As a counterpoint, I really don't like this because HashMap
is a very standard name, and using utils::HashMap
(which usually gets imported through use utils::*
) is generally confusing.
As it turns out, utils::HashMap
is nothing else than fnv::FnvHashMap
(from the fnv
crate, which telamon-utils
uses), and I would be very much in favour of renaming our use of HashMap
into this instead (which I think I'll actually do 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.
can you use utils::FnvHashMap instead ?
d867cdf
to
d233633
Compare
|
||
trait ReplaceDurationIfLower { | ||
/// Set duration to d if currently undefined or if d is lower | ||
fn replace_if_lower(&mut self, d: Duration); |
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.
Nit: Using a trait for this instead of a free-standing function is a bit overkill, but OK.
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.
OK with me if you remove the AddAssign
and Sub
implementations on EdgeInde
.
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.
minor changes.
src/offline_analysis/tree.rs
Outdated
///! candidate tree from a log file | ||
use crate::explorer::choice::ActionEx as Action; | ||
use std::cell::RefCell; | ||
use std::collections::HashMap; |
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.
can you use utils::FnvHashMap instead ?
b45361f
to
6f8a36f
Compare
The new module offline_analysis allows for the reconstruction of a candidate tree from the events of a log file for post-mortem analysis or conversion to a different log format required by an analysis tool. The reconstruction of the tree itself and refinement of candidate nodes from actions on the nodes can be done incrementally and in an interleaved fashion as log messages are processed. The general outline of this process is: let mut tree = CandidateTree::new(); // Iterate over all messages from the log file for ... { match message { Message::Node { ... } => { tree.extend(...); } Message::Trace { ... } => { // Iterate over all events of the trace message for event ... { match event { Event::Kill(...) => { // Declare candidate as a deadend tree.get_node(curr_node_id).declare_deadend(...); ... } Event::Implementation(...) => { // Declare candidate as implementation (fully specified candidate) tree.get_node(curr_node_id).declare_implementation(...); ... } ... } } } Message::Evaluation { ... } => { ... // Update score t.get_node(...).set_score(...); } } }
bors r=ulysseB, r=Elarnon |
212: Add code for off-line reconstruction of candidate trees r=ulysseB,r=Elarnon a=andidr The new module offline_analysis allows for the reconstruction of a candidate tree from the events of a log file for post-mortem analysis or conversion to a different log format required by an analysis tool. The reconstruction of the tree itself and refinement of candidate nodes from actions on the nodes can be done incrementally and in an interleaved fashion as log messages are processed. The general outline of this process is: let mut tree = CandidateTree::new(); let mut actions = ThreadActionList::new(); // Iterate over all messages from the log file for ... { match message { Message::Node { ... } => { tree.extend(...); } Message::Trace { ... } => { // Iterate over all events of the trace message for event ... { match event { Event::Kill(...) => { // Declare candidate as a deadend tree.get_node(curr_node_id).borrow_mut().declare_deadend(...); ... } Event::Implementation(...) => { // Update score tree.get_node(curr_node_id) .borrow_mut() .declare_implementation(...); ... } ... } } } Message::Evaluation { ... } => { ... t.get_node(..).borrow_mut().set_score(...); } } } Co-authored-by: Andi Drebes <andi@drebesium.org>
Build failed |
bors retry |
212: Add code for off-line reconstruction of candidate trees r=ulysseB,r=Elarnon a=andidr The new module offline_analysis allows for the reconstruction of a candidate tree from the events of a log file for post-mortem analysis or conversion to a different log format required by an analysis tool. The reconstruction of the tree itself and refinement of candidate nodes from actions on the nodes can be done incrementally and in an interleaved fashion as log messages are processed. The general outline of this process is: let mut tree = CandidateTree::new(); let mut actions = ThreadActionList::new(); // Iterate over all messages from the log file for ... { match message { Message::Node { ... } => { tree.extend(...); } Message::Trace { ... } => { // Iterate over all events of the trace message for event ... { match event { Event::Kill(...) => { // Declare candidate as a deadend tree.get_node(curr_node_id).borrow_mut().declare_deadend(...); ... } Event::Implementation(...) => { // Update score tree.get_node(curr_node_id) .borrow_mut() .declare_implementation(...); ... } ... } } } Message::Evaluation { ... } => { ... t.get_node(..).borrow_mut().set_score(...); } } } Co-authored-by: Andi Drebes <andi@drebesium.org>
Timed out |
bors retry |
212: Add code for off-line reconstruction of candidate trees r=ulysseB,r=Elarnon a=andidr The new module offline_analysis allows for the reconstruction of a candidate tree from the events of a log file for post-mortem analysis or conversion to a different log format required by an analysis tool. The reconstruction of the tree itself and refinement of candidate nodes from actions on the nodes can be done incrementally and in an interleaved fashion as log messages are processed. The general outline of this process is: let mut tree = CandidateTree::new(); let mut actions = ThreadActionList::new(); // Iterate over all messages from the log file for ... { match message { Message::Node { ... } => { tree.extend(...); } Message::Trace { ... } => { // Iterate over all events of the trace message for event ... { match event { Event::Kill(...) => { // Declare candidate as a deadend tree.get_node(curr_node_id).borrow_mut().declare_deadend(...); ... } Event::Implementation(...) => { // Update score tree.get_node(curr_node_id) .borrow_mut() .declare_implementation(...); ... } ... } } } Message::Evaluation { ... } => { ... t.get_node(..).borrow_mut().set_score(...); } } } Co-authored-by: Andi Drebes <andi@drebesium.org>
Build succeeded |
The new module offline_analysis allows for the reconstruction of a candidate
tree from the events of a log file for post-mortem analysis or conversion to a
different log format required by an analysis tool.
The reconstruction of the tree itself and refinement of candidate nodes from
actions on the nodes can be done incrementally and in an interleaved fashion as
log messages are processed. The general outline of this process is:
let mut tree = CandidateTree::new();
let mut actions = ThreadActionList::new();
// Iterate over all messages from the log file
for ... {
match message {
Message::Node { ... } => { tree.extend(...); }
}