Skip to content

Total & remaining rows, and csv output for diff. #98

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

Merged
merged 1 commit into from
Jul 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
150 changes: 108 additions & 42 deletions analyze/analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#![deny(missing_docs)]
#![deny(missing_debug_implementations)]

extern crate serde;
#[macro_use]
extern crate serde_derive;
extern crate csv;
Expand All @@ -14,8 +15,9 @@ extern crate twiggy_traits as traits;

mod json;

use serde::ser::SerializeStruct;
use std::cmp;
use std::collections::{BTreeMap, BTreeSet};
use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
use std::fmt;
use std::io;

Expand Down Expand Up @@ -1119,6 +1121,18 @@ impl Ord for DiffEntry {
}
}

impl serde::Serialize for DiffEntry {
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to implement this by hand rather than to use #[derive(Serialize)]?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah! This was implemented manually because the delta value should have its sign shown even if it is positive. The derived implementation matched tests for the negative values, but positive values were missing the +.

If there's a way to do this with serde macros though, I'm happy to follow up and fix that :)

fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
let mut state = serializer.serialize_struct("DiffEntry", 2)?;
state.serialize_field("DeltaBytes", &format!("{:+}", self.delta))?;
state.serialize_field("Item", &self.name)?;
state.end()
}
}

impl traits::Emit for Diff {
#[cfg(feature = "emit_text")]
fn emit_text(
Expand All @@ -1131,9 +1145,10 @@ impl traits::Emit for Diff {
(Align::Left, "Item".to_string()),
]);

for entry in &self.deltas {
table.add_row(vec![format!("{:+}", entry.delta), entry.name.clone()]);
}
self.deltas
.iter()
.map(|entry| vec![format!("{:+}", entry.delta), entry.name.clone()])
.for_each(|row| table.add_row(row));

write!(dest, "{}", &table)?;
Ok(())
Expand All @@ -1147,7 +1162,7 @@ impl traits::Emit for Diff {
) -> Result<(), traits::Error> {
let mut arr = json::array(dest)?;

for entry in &self.deltas {
for entry in self.deltas.iter() {
let mut obj = arr.object()?;
obj.field("delta_bytes", entry.delta as f64)?;
obj.field("name", entry.name.as_str())?;
Expand All @@ -1157,8 +1172,15 @@ impl traits::Emit for Diff {
}

#[cfg(feature = "emit_csv")]
fn emit_csv(&self, _items: &ir::Items, _dest: &mut io::Write) -> Result<(), traits::Error> {
unimplemented!();
fn emit_csv(&self, _items: &ir::Items, dest: &mut io::Write) -> Result<(), traits::Error> {
let mut wtr = csv::Writer::from_writer(dest);

for entry in self.deltas.iter() {
wtr.serialize(entry)?;
wtr.flush()?;
}

Ok(())
}
}

Expand All @@ -1168,48 +1190,92 @@ pub fn diff(
new_items: &mut ir::Items,
opts: &opt::Diff,
) -> Result<Box<traits::Emit>, traits::Error> {
let old_items_by_name: BTreeMap<&str, &ir::Item> =
old_items.iter().map(|item| (item.name(), item)).collect();
let new_items_by_name: BTreeMap<&str, &ir::Item> =
new_items.iter().map(|item| (item.name(), item)).collect();

let mut deltas = vec![];

for (name, old_item) in &old_items_by_name {
match new_items_by_name.get(name) {
None => deltas.push(DiffEntry {
name: name.to_string(),
delta: -(old_item.size() as i64),
}),
Some(new_item) => {
let delta = new_item.size() as i64 - old_item.size() as i64;
if delta != 0 {
deltas.push(DiffEntry {
name: name.to_string(),
delta,
});
}
}
}
let max_items = opts.max_items() as usize;

// Given a set of items, create a HashMap of the items' names and sizes.
fn get_names_and_sizes(items: &ir::Items) -> HashMap<&str, i64> {
items
.iter()
.map(|item| (item.name(), item.size() as i64))
.collect()
}

for (name, new_item) in &new_items_by_name {
if !old_items_by_name.contains_key(name) {
deltas.push(DiffEntry {
name: name.to_string(),
delta: new_item.size() as i64,
});
// Collect the names and sizes of the items in the old and new collections.
let old_sizes = get_names_and_sizes(old_items);
let new_sizes = get_names_and_sizes(new_items);

// Given an item name, create a `DiffEntry` object representing the
// change in size, or an error if the name could not be found in
// either of the item collections.
let get_item_delta = |name: String| -> Result<DiffEntry, traits::Error> {
let old_size = old_sizes.get::<str>(&name);
let new_size = new_sizes.get::<str>(&name);
let delta: i64 = match (old_size, new_size) {
(Some(old_size), Some(new_size)) => new_size - old_size,
(Some(old_size), None) => -old_size,
(None, Some(new_size)) => *new_size,
(None, None) => {
return Err(traits::Error::with_msg(format!(
"Could not find item with name `{}`",
name
)))
}
};
Ok(DiffEntry { name, delta })
};

// Given a result returned by `get_item_delta`, return false if the result
// represents an unchanged item. Ignore errors, these are handled separately.
let unchanged_items_filter = |res: &Result<DiffEntry, traits::Error>| -> bool {
if let Ok(DiffEntry { name: _, delta: 0 }) = res {
false
} else {
true
}
}
};

deltas.push(DiffEntry {
name: "<total>".to_string(),
delta: new_items.size() as i64 - old_items.size() as i64,
});
// Create a set of item names from the new and old item collections.
let names = old_sizes
.keys()
.chain(new_sizes.keys())
.map(|k| k.to_string())
.collect::<HashSet<_>>();

// Iterate through the set of item names, and use the closure above to map
// each item into a `DiffEntry` object. Then, sort the collection.
let mut deltas = names
.into_iter()
.map(get_item_delta)
.filter(unchanged_items_filter)
.collect::<Result<Vec<_>, traits::Error>>()?;
deltas.sort();
deltas.truncate(opts.max_items() as usize);

// Create an entry to summarize the diff rows that will be truncated.
let (rem_cnt, rem_delta): (u32, i64) = deltas
.iter()
.skip(max_items)
.fold((0, 0), |(cnt, rem_delta), DiffEntry { name: _, delta }| {
(cnt + 1, rem_delta + delta)
});
let remaining = DiffEntry {
name: format!("... and {} more.", rem_cnt),
delta: rem_delta,
};

// Create a `DiffEntry` representing the net change, and total row count.
let total = DiffEntry {
name: format!("Σ [{} Total Rows]", deltas.len()),
delta: new_items.size() as i64 - old_items.size() as i64,
};

// Now that the 'remaining' and 'total' summary entries have been created,
// truncate the vector of deltas before we box up the result, and push
// the remaining and total rows to the deltas vector.
deltas.truncate(max_items);
deltas.push(remaining);
deltas.push(total);

// Return the results so that they can be emitted.
let diff = Diff { deltas };
Ok(Box::new(diff) as Box<traits::Emit>)
}
Expand Down
4 changes: 3 additions & 1 deletion twiggy/tests/expectations/diff_wee_alloc
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
Delta Bytes │ Item
─────────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
-1476 ┊ <total>
-1034 ┊ data[3]
-593 ┊ "function names" subsection
+395 ┊ wee_alloc::alloc_first_fit::he2a4ddf96981c0ce
Expand All @@ -20,3 +19,6 @@
-8 ┊ type[4]
-6 ┊ <wee_alloc::LargeAllocPolicy as wee_alloc::AllocPolicy>::min_cell_size::hc7cee2a550987099
+6 ┊ alloc::alloc::oom::h45ae3f22a516fb04
-5 ┊ <wee_alloc::size_classes::SizeClassAllocPolicy<'a> as wee_alloc::AllocPolicy>::min_cell_size::h6f746be886573355
-21 ┊ ... and 13 more.
-1476 ┊ Σ [33 Total Rows]
23 changes: 23 additions & 0 deletions twiggy/tests/expectations/diff_wee_alloc_csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
DeltaBytes,Item
-1034,data[3]
-593,"""function names"" subsection"
+395,wee_alloc::alloc_first_fit::he2a4ddf96981c0ce
+243,goodbye
-225,wee_alloc::alloc_first_fit::h9a72de3af77ef93f
-152,wee_alloc::alloc_with_refill::hb32c1bbce9ebda8e
+145,"<wee_alloc::neighbors::Neighbors<'a, T>>::remove::hc9e5d4284e8233b8"
-136,<wee_alloc::size_classes::SizeClassAllocPolicy<'a> as wee_alloc::AllocPolicy>::new_cell_for_free_list::h3987e3054b8224e6
-76,<wee_alloc::LargeAllocPolicy as wee_alloc::AllocPolicy>::new_cell_for_free_list::h8f071b7bce0301ba
-25,data[1]
-25,data[2]
+15,hello
+15,import env::rust_oom
+12,custom section 'linking'
-12,elem[0]
+8,global[0]
-8,type[4]
-6,<wee_alloc::LargeAllocPolicy as wee_alloc::AllocPolicy>::min_cell_size::hc7cee2a550987099
+6,alloc::alloc::oom::h45ae3f22a516fb04
-5,<wee_alloc::size_classes::SizeClassAllocPolicy<'a> as wee_alloc::AllocPolicy>::min_cell_size::h6f746be886573355
-21,... and 13 more.
-1476,Σ [33 Total Rows]
8 changes: 8 additions & 0 deletions twiggy/tests/expectations/diff_wee_alloc_csv_top_5
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
DeltaBytes,Item
-1034,data[3]
-593,"""function names"" subsection"
+395,wee_alloc::alloc_first_fit::he2a4ddf96981c0ce
+243,goodbye
-225,wee_alloc::alloc_first_fit::h9a72de3af77ef93f
-265,... and 28 more.
-1476,Σ [33 Total Rows]
2 changes: 1 addition & 1 deletion twiggy/tests/expectations/diff_wee_alloc_json
Original file line number Diff line number Diff line change
@@ -1 +1 @@
[{"delta_bytes":-1476,"name":"<total>"},{"delta_bytes":-1034,"name":"data[3]"},{"delta_bytes":-593,"name":"\"function names\" subsection"},{"delta_bytes":395,"name":"wee_alloc::alloc_first_fit::he2a4ddf96981c0ce"},{"delta_bytes":243,"name":"goodbye"}]
[{"delta_bytes":-1034,"name":"data[3]"},{"delta_bytes":-593,"name":"\"function names\" subsection"},{"delta_bytes":395,"name":"wee_alloc::alloc_first_fit::he2a4ddf96981c0ce"},{"delta_bytes":243,"name":"goodbye"},{"delta_bytes":-225,"name":"wee_alloc::alloc_first_fit::h9a72de3af77ef93f"},{"delta_bytes":-152,"name":"wee_alloc::alloc_with_refill::hb32c1bbce9ebda8e"},{"delta_bytes":145,"name":"<wee_alloc::neighbors::Neighbors<'a, T>>::remove::hc9e5d4284e8233b8"},{"delta_bytes":-136,"name":"<wee_alloc::size_classes::SizeClassAllocPolicy<'a> as wee_alloc::AllocPolicy>::new_cell_for_free_list::h3987e3054b8224e6"},{"delta_bytes":-76,"name":"<wee_alloc::LargeAllocPolicy as wee_alloc::AllocPolicy>::new_cell_for_free_list::h8f071b7bce0301ba"},{"delta_bytes":-25,"name":"data[1]"},{"delta_bytes":-25,"name":"data[2]"},{"delta_bytes":15,"name":"hello"},{"delta_bytes":15,"name":"import env::rust_oom"},{"delta_bytes":12,"name":"custom section 'linking'"},{"delta_bytes":-12,"name":"elem[0]"},{"delta_bytes":8,"name":"global[0]"},{"delta_bytes":-8,"name":"type[4]"},{"delta_bytes":-6,"name":"<wee_alloc::LargeAllocPolicy as wee_alloc::AllocPolicy>::min_cell_size::hc7cee2a550987099"},{"delta_bytes":6,"name":"alloc::alloc::oom::h45ae3f22a516fb04"},{"delta_bytes":-5,"name":"<wee_alloc::size_classes::SizeClassAllocPolicy<'a> as wee_alloc::AllocPolicy>::min_cell_size::h6f746be886573355"},{"delta_bytes":-21,"name":"... and 13 more."},{"delta_bytes":-1476,"name":"Σ [33 Total Rows]"}]
1 change: 1 addition & 0 deletions twiggy/tests/expectations/diff_wee_alloc_json_top_5
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[{"delta_bytes":-1034,"name":"data[3]"},{"delta_bytes":-593,"name":"\"function names\" subsection"},{"delta_bytes":395,"name":"wee_alloc::alloc_first_fit::he2a4ddf96981c0ce"},{"delta_bytes":243,"name":"goodbye"},{"delta_bytes":-225,"name":"wee_alloc::alloc_first_fit::h9a72de3af77ef93f"},{"delta_bytes":-265,"name":"... and 28 more."},{"delta_bytes":-1476,"name":"Σ [33 Total Rows]"}]
9 changes: 9 additions & 0 deletions twiggy/tests/expectations/diff_wee_alloc_top_5
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Delta Bytes │ Item
─────────────┼──────────────────────────────────────────────
-1034 ┊ data[3]
-593 ┊ "function names" subsection
+395 ┊ wee_alloc::alloc_first_fit::he2a4ddf96981c0ce
+243 ┊ goodbye
-225 ┊ wee_alloc::alloc_first_fit::h9a72de3af77ef93f
-265 ┊ ... and 28 more.
-1476 ┊ Σ [33 Total Rows]
38 changes: 38 additions & 0 deletions twiggy/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,17 +352,55 @@ test!(
"./fixtures/wee_alloc.2.wasm"
);

test!(
diff_wee_alloc_top_5,
"diff",
"./fixtures/wee_alloc.wasm",
"./fixtures/wee_alloc.2.wasm",
"-n",
"5"
);

test!(
diff_wee_alloc_json,
"diff",
"./fixtures/wee_alloc.wasm",
"./fixtures/wee_alloc.2.wasm",
"-f",
"json"
);

test!(
diff_wee_alloc_json_top_5,
"diff",
"./fixtures/wee_alloc.wasm",
"./fixtures/wee_alloc.2.wasm",
"-f",
"json",
"-n",
"5"
);

test!(
diff_wee_alloc_csv,
"diff",
"./fixtures/wee_alloc.wasm",
"./fixtures/wee_alloc.2.wasm",
"-f",
"csv"
);

test!(
diff_wee_alloc_csv_top_5,
"diff",
"./fixtures/wee_alloc.wasm",
"./fixtures/wee_alloc.2.wasm",
"-f",
"csv",
"-n",
"5"
);

test!(garbage, "garbage", "./fixtures/garbage.wasm");

test!(
Expand Down