From 863ed19946d6f707ce09dd77bf26b26be73e097c Mon Sep 17 00:00:00 2001 From: Bernardo Date: Mon, 24 Dec 2018 17:01:25 +0100 Subject: remove benchmark and simplify tests --- crates/ra_editor/Cargo.toml | 12 --- .../translate_offset_with_edit_benchmark.rs | 87 ---------------------- crates/ra_editor/src/line_index.rs | 23 +++++- crates/ra_editor/src/line_index_utils.rs | 63 +++++----------- crates/ra_text_edit/src/test_utils.rs | 72 +++++------------- crates/ra_text_edit/src/text_edit.rs | 15 ++-- 6 files changed, 69 insertions(+), 203 deletions(-) delete mode 100644 crates/ra_editor/benches/translate_offset_with_edit_benchmark.rs (limited to 'crates') diff --git a/crates/ra_editor/Cargo.toml b/crates/ra_editor/Cargo.toml index 7ed7526ec..1ad99af28 100644 --- a/crates/ra_editor/Cargo.toml +++ b/crates/ra_editor/Cargo.toml @@ -18,15 +18,3 @@ proptest = "0.8.7" [dev-dependencies] test_utils = { path = "../test_utils" } -criterion = "0.2" -rand = "*" -rand_xorshift = "*" -lazy_static = "*" - -[lib] -# so that criterion arguments work, see: https://bheisler.github.io/criterion.rs/book/faq.html#cargo-bench-gives-unrecognized-option-errors-for-valid-command-line-options -bench = false - -[[bench]] -name = "translate_offset_with_edit_benchmark" -harness = false \ No newline at end of file diff --git a/crates/ra_editor/benches/translate_offset_with_edit_benchmark.rs b/crates/ra_editor/benches/translate_offset_with_edit_benchmark.rs deleted file mode 100644 index 0f550fd39..000000000 --- a/crates/ra_editor/benches/translate_offset_with_edit_benchmark.rs +++ /dev/null @@ -1,87 +0,0 @@ -use criterion::{criterion_group, criterion_main}; -use criterion::Criterion; -use criterion::Fun; -use ra_text_edit::AtomTextEdit; -use ra_text_edit::test_utils::{arb_edits_custom, arb_offset}; -use ra_editor::line_index_utils; -use ra_editor::LineIndex; -use ra_syntax::TextUnit; -use proptest::test_runner; -use proptest::string::string_regex; -use proptest::strategy::{Strategy, ValueTree}; -use rand_xorshift::XorShiftRng; -use rand::SeedableRng; -use lazy_static::lazy_static; - -#[derive(Debug)] -struct Data { - text: String, - line_index: LineIndex, - edits: Vec, - offset: TextUnit, -} - -fn setup_data() -> Data { - let mut runner = test_runner::TestRunner::default(); - { - struct TestRng { - rng: XorShiftRng, - } - // HACK to be able to manually seed the TestRunner - let rng: &mut TestRng = unsafe { std::mem::transmute(runner.rng()) }; - rng.rng = XorShiftRng::seed_from_u64(0); - } - - let text = { - let arb = string_regex("([a-zA-Z_0-9]{10,50}.{1,5}\n){100,500}").unwrap(); - let tree = arb.new_tree(&mut runner).unwrap(); - tree.current() - }; - - let edits = { - let arb = arb_edits_custom(&text, 99, 100); - let tree = arb.new_tree(&mut runner).unwrap(); - tree.current() - }; - - let offset = { - let arb = arb_offset(&text); - let tree = arb.new_tree(&mut runner).unwrap(); - tree.current() - }; - - let line_index = LineIndex::new(&text); - - Data { - text, - line_index, - edits, - offset, - } -} - -lazy_static! { - static ref DATA: Data = setup_data(); -} - -fn compare_translates(c: &mut Criterion) { - let functions = vec![ - Fun::new("translate_after_edit", |b, _| { - b.iter(|| { - let d = &*DATA; - line_index_utils::translate_after_edit(&d.text, d.offset, d.edits.clone()); - }) - }), - Fun::new("translate_offset_with_edit", |b, _| { - b.iter(|| { - let d = &*DATA; - line_index_utils::translate_offset_with_edit(&d.line_index, d.offset, &d.edits); - }) - }), - ]; - - c.bench_functions("translate", functions, ()); -} - -criterion_group!(benches, compare_translates); -criterion_main!(benches); diff --git a/crates/ra_editor/src/line_index.rs b/crates/ra_editor/src/line_index.rs index 5304fbcf6..898fee7e0 100644 --- a/crates/ra_editor/src/line_index.rs +++ b/crates/ra_editor/src/line_index.rs @@ -128,8 +128,8 @@ impl LineIndex { } } +#[cfg(test)] /// Simple reference implementation to use in proptests -/// and benchmarks as baseline pub fn to_line_col(text: &str, offset: TextUnit) -> LineCol { let mut res = LineCol { line: 0, @@ -270,6 +270,27 @@ mod test_line_index { .boxed() } + fn to_line_col(text: &str, offset: TextUnit) -> LineCol { + let mut res = LineCol { + line: 0, + col_utf16: 0, + }; + for (i, c) in text.char_indices() { + if i + c.len_utf8() > offset.to_usize() { + // if it's an invalid offset, inside a multibyte char + // return as if it was at the start of the char + break; + } + if c == '\n' { + res.line += 1; + res.col_utf16 = 0; + } else { + res.col_utf16 += 1; + } + } + res + } + proptest! { #[test] fn test_line_index_proptest((offset, text) in arb_text_with_offset()) { diff --git a/crates/ra_editor/src/line_index_utils.rs b/crates/ra_editor/src/line_index_utils.rs index ba3ac8aeb..b8b149442 100644 --- a/crates/ra_editor/src/line_index_utils.rs +++ b/crates/ra_editor/src/line_index_utils.rs @@ -1,6 +1,6 @@ use ra_text_edit::AtomTextEdit; use ra_syntax::{TextUnit, TextRange}; -use crate::{LineIndex, LineCol, line_index::{self, Utf16Char}}; +use crate::{LineIndex, LineCol, line_index::Utf16Char}; use superslice::Ext; #[derive(Debug, Clone)] @@ -325,59 +325,34 @@ pub fn translate_offset_with_edit( res.to_line_col(offset) } -/// Simplest implementation to use as reference in proptest and benchmarks -pub fn translate_after_edit( - pre_edit_text: &str, - offset: TextUnit, - edits: Vec, -) -> LineCol { - let text = edit_text(pre_edit_text, edits); - line_index::to_line_col(&text, offset) -} - -fn edit_text(pre_edit_text: &str, mut edits: Vec) -> String { - // apply edits ordered from last to first - // since they should not overlap we can just use start() - edits.sort_by_key(|x| -(x.delete.start().to_usize() as isize)); - - let mut text = pre_edit_text.to_owned(); - - for edit in &edits { - let range = edit.delete.start().to_usize()..edit.delete.end().to_usize(); - text.replace_range(range, &edit.insert); - } - - text -} - #[cfg(test)] mod test { use super::*; use proptest::{prelude::*, proptest, proptest_helper}; - use ra_text_edit::test_utils::{arb_text, arb_offset, arb_edits}; + use crate::line_index; + use ra_text_edit::test_utils::{arb_offset, arb_text_with_edits}; + use ra_text_edit::TextEdit; #[derive(Debug)] struct ArbTextWithOffsetAndEdits { text: String, + edits: TextEdit, edited_text: String, offset: TextUnit, - edits: Vec, } - fn arb_text_with_offset_and_edits() -> BoxedStrategy { - arb_text() - .prop_flat_map(|text| { - (arb_edits(&text), Just(text)).prop_flat_map(|(edits, text)| { - let edited_text = edit_text(&text, edits.clone()); - let arb_offset = arb_offset(&edited_text); - (Just(text), Just(edited_text), Just(edits), arb_offset).prop_map( - |(text, edited_text, edits, offset)| ArbTextWithOffsetAndEdits { - text, - edits, - edited_text, - offset, - }, - ) + fn arb_text_with_edits_and_offset() -> BoxedStrategy { + arb_text_with_edits() + .prop_flat_map(|x| { + let edited_text = x.edits.apply(&x.text); + let arb_offset = arb_offset(&edited_text); + (Just(x), Just(edited_text), arb_offset).prop_map(|(x, edited_text, offset)| { + ArbTextWithOffsetAndEdits { + text: x.text, + edits: x.edits, + edited_text, + offset, + } }) }) .boxed() @@ -385,10 +360,10 @@ mod test { proptest! { #[test] - fn test_translate_offset_with_edit(x in arb_text_with_offset_and_edits()) { + fn test_translate_offset_with_edit(x in arb_text_with_edits_and_offset()) { let expected = line_index::to_line_col(&x.edited_text, x.offset); let line_index = LineIndex::new(&x.text); - let actual = translate_offset_with_edit(&line_index, x.offset, &x.edits); + let actual = translate_offset_with_edit(&line_index, x.offset, x.edits.as_atoms()); assert_eq!(actual, expected); } diff --git a/crates/ra_text_edit/src/test_utils.rs b/crates/ra_text_edit/src/test_utils.rs index f150288f6..f0b8dfde1 100644 --- a/crates/ra_text_edit/src/test_utils.rs +++ b/crates/ra_text_edit/src/test_utils.rs @@ -1,6 +1,6 @@ use proptest::prelude::*; use text_unit::{TextUnit, TextRange}; -use crate::AtomTextEdit; +use crate::{AtomTextEdit, TextEdit}; pub fn arb_text() -> proptest::string::RegexGeneratorStrategy { // generate multiple newlines @@ -23,11 +23,7 @@ pub fn arb_offset(text: &str) -> BoxedStrategy { } } -pub fn arb_edits(text: &str) -> BoxedStrategy> { - arb_edits_custom(&text, 0, 7) -} - -pub fn arb_edits_custom(text: &str, min: usize, max: usize) -> BoxedStrategy> { +pub fn arb_text_edit(text: &str) -> BoxedStrategy { if text.is_empty() { // only valid edits return Just(vec![]) @@ -37,14 +33,14 @@ pub fn arb_edits_custom(text: &str, min: usize, max: usize) -> BoxedStrategy = cuts .chunks(2) @@ -68,52 +64,22 @@ pub fn arb_edits_custom(text: &str, min: usize, max: usize) -> BoxedStrategy BoxedStrategy<(String, Vec)> { - let text = arb_text(); - text.prop_flat_map(|s| { - let edits = arb_edits(&s); - (Just(s), edits) - }) - .boxed() - } - - fn intersect(r1: TextRange, r2: TextRange) -> Option { - let start = r1.start().max(r2.start()); - let end = r1.end().min(r2.end()); - if start <= end { - Some(TextRange::from_to(start, end)) - } else { - None - } - } - proptest! { - #[test] - fn atom_text_edits_are_valid((text, edits) in arb_text_with_edits()) { - proptest_atom_text_edits_are_valid(text, edits) - } - } +#[derive(Debug, Clone)] +pub struct ArbTextWithEdits { + pub text: String, + pub edits: TextEdit, +} - fn proptest_atom_text_edits_are_valid(text: String, edits: Vec) { - // slicing doesn't panic - for e in &edits { - let _ = &text[e.delete]; - } - // ranges do not overlap - for i in 1..edits.len() { - let e1 = &edits[i]; - for e2 in &edits[0..i] { - if intersect(e1.delete, e2.delete).is_some() { - assert!(false, "Overlapping ranges {} {}", e1.delete, e2.delete); - } - } - } - } +pub fn arb_text_with_edits() -> BoxedStrategy { + let text = arb_text(); + text.prop_flat_map(|s| { + let edits = arb_text_edit(&s); + (Just(s), edits) + }) + .prop_map(|(text, edits)| ArbTextWithEdits { text, edits }) + .boxed() } diff --git a/crates/ra_text_edit/src/text_edit.rs b/crates/ra_text_edit/src/text_edit.rs index 392968d63..0881f3e1c 100644 --- a/crates/ra_text_edit/src/text_edit.rs +++ b/crates/ra_text_edit/src/text_edit.rs @@ -26,12 +26,7 @@ impl TextEditBuilder { self.atoms.push(AtomTextEdit::insert(offset, text)) } pub fn finish(self) -> TextEdit { - let mut atoms = self.atoms; - atoms.sort_by_key(|a| (a.delete.start(), a.delete.end())); - for (a1, a2) in atoms.iter().zip(atoms.iter().skip(1)) { - assert!(a1.delete.end() <= a2.delete.start()) - } - TextEdit { atoms } + TextEdit::from_atoms(self.atoms) } pub fn invalidates_offset(&self, offset: TextUnit) -> bool { self.atoms @@ -41,6 +36,14 @@ impl TextEditBuilder { } impl TextEdit { + pub(crate) fn from_atoms(mut atoms: Vec) -> TextEdit { + atoms.sort_by_key(|a| (a.delete.start(), a.delete.end())); + for (a1, a2) in atoms.iter().zip(atoms.iter().skip(1)) { + assert!(a1.delete.end() <= a2.delete.start()) + } + TextEdit { atoms } + } + pub fn as_atoms(&self) -> &[AtomTextEdit] { &self.atoms } -- cgit v1.2.3