Skip to content

Commit

Permalink
api: deprecate byte classes and premultiply options
Browse files Browse the repository at this point in the history
These options aren't really carrying their weight. In a future release,
aho-corasick will make both options enabled by default all the time with
the impossibility of disabling them. The main reason for this is that
these options require a quadrupling in the amount of code in this crate.

While it's possible to see a performance hit when using byte classes, it
should generally be very small. The improvement, if one exists, just
doesn't see worth it.

Please see #57 for more
discussion.

This is meant to mirror a similar decision occurring in regex-automata:
BurntSushi/regex-automata#7.
  • Loading branch information
BurntSushi committed Apr 30, 2021
1 parent 26cd2ac commit adc8c6a
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 13 deletions.
2 changes: 2 additions & 0 deletions aho-corasick-debug/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ impl Args {
eprintln!("pattern read time: {:?}", read_time);

let start = Instant::now();
// TODO: remove when byte classes and premultiply options are removed.
#[allow(deprecated)]
let ac = AhoCorasickBuilder::new()
.match_kind(self.match_kind)
.ascii_case_insensitive(self.ascii_casei)
Expand Down
22 changes: 9 additions & 13 deletions src/ahocorasick.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1005,18 +1005,6 @@ impl<S: StateID> AhoCorasick<S> {
///
/// let ac = AhoCorasickBuilder::new()
/// .dfa(true)
/// .byte_classes(false)
/// .build(&["foo", "bar", "baz"]);
/// assert_eq!(20_768, ac.heap_bytes());
///
/// let ac = AhoCorasickBuilder::new()
/// .dfa(true)
/// .byte_classes(true) // default
/// .build(&["foo", "bar", "baz"]);
/// assert_eq!(1_248, ac.heap_bytes());
///
/// let ac = AhoCorasickBuilder::new()
/// .dfa(true)
/// .ascii_case_insensitive(true)
/// .build(&["foo", "bar", "baz"]);
/// assert_eq!(1_248, ac.heap_bytes());
Expand Down Expand Up @@ -1681,7 +1669,7 @@ impl AhoCorasickBuilder {
// N.B. Using byte classes can actually be faster by improving
// locality, but this only really applies for multi-megabyte
// automata (i.e., automata that don't fit in your CPU's cache).
self.dfa(true).byte_classes(false);
self.dfa(true);
} else if patterns.len() <= 5000 {
self.dfa(true);
}
Expand Down Expand Up @@ -1928,6 +1916,10 @@ impl AhoCorasickBuilder {
/// overall performance.
///
/// This option is enabled by default.
#[deprecated(
since = "0.7.16",
note = "not carrying its weight, will be always enabled, see: https://github.com/BurntSushi/aho-corasick/issues/57"
)]
pub fn byte_classes(&mut self, yes: bool) -> &mut AhoCorasickBuilder {
self.dfa_builder.byte_classes(yes);
self
Expand Down Expand Up @@ -1956,6 +1948,10 @@ impl AhoCorasickBuilder {
/// non-premultiplied form only requires 8 bits.
///
/// This option is enabled by default.
#[deprecated(
since = "0.7.16",
note = "not carrying its weight, will be always enabled, see: https://github.com/BurntSushi/aho-corasick/issues/57"
)]
pub fn premultiply(&mut self, yes: bool) -> &mut AhoCorasickBuilder {
self.dfa_builder.premultiply(yes);
self
Expand Down
12 changes: 12 additions & 0 deletions src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,8 @@ macro_rules! testcombo {
$collection,
$kind,
|b: &mut AhoCorasickBuilder| {
// TODO: remove tests when option is removed.
#[allow(deprecated)]
b.dfa(true).byte_classes(false);
}
);
Expand All @@ -747,6 +749,8 @@ macro_rules! testcombo {
$collection,
$kind,
|b: &mut AhoCorasickBuilder| {
// TODO: remove tests when option is removed.
#[allow(deprecated)]
b.dfa(true).premultiply(false);
}
);
Expand All @@ -755,6 +759,8 @@ macro_rules! testcombo {
$collection,
$kind,
|b: &mut AhoCorasickBuilder| {
// TODO: remove tests when options are removed.
#[allow(deprecated)]
b.dfa(true).byte_classes(false).premultiply(false);
}
);
Expand Down Expand Up @@ -830,6 +836,8 @@ testconfig!(
AC_STANDARD_OVERLAPPING,
Standard,
|b: &mut AhoCorasickBuilder| {
// TODO: remove tests when option is removed.
#[allow(deprecated)]
b.dfa(true).byte_classes(false);
}
);
Expand All @@ -839,6 +847,8 @@ testconfig!(
AC_STANDARD_OVERLAPPING,
Standard,
|b: &mut AhoCorasickBuilder| {
// TODO: remove tests when option is removed.
#[allow(deprecated)]
b.dfa(true).premultiply(false);
}
);
Expand All @@ -848,6 +858,8 @@ testconfig!(
AC_STANDARD_OVERLAPPING,
Standard,
|b: &mut AhoCorasickBuilder| {
// TODO: remove tests when options are removed.
#[allow(deprecated)]
b.dfa(true).byte_classes(false).premultiply(false);
}
);
Expand Down

0 comments on commit adc8c6a

Please sign in to comment.