Release the GIL for BytesAhoCorasick when the haystack is bytes

In theory we should do that for every type that is guaranteed to be
immutable, in order to improve performance, but there's no way to tell
whether a type is immutable or not through the Buffer protocol, so we
only do this as a special case for `bytes`.

Fixes #94.

README.md

@@ -193,7 +193,7 @@ You can control the behavior by using the `store_patterns` keyword argument to `
 ## Implementation details <a name="implementation"></a>
 
 * Matching on strings releases the GIL, to enable concurrency.
-  Matching on bytes does not currently release the GIL, but see https://github.com/G-Research/ahocorasick_rs/issues/94 for a case where it could.
+  Matching on bytes does not currently release the GIL for memory-safety reasons, unless the haystack type is `bytes`.
 * Not all features from the underlying library are exposed; if you would like additional features, please [file an issue](https://github.com/g-research/ahocorasick_rs/issues/new) or submit a PR.
 
 ## Benchmarks <a name="benchmarks"></a>

src/lib.rs

@@ -8,7 +8,7 @@ use pyo3::{
     buffer::{PyBuffer, ReadOnlyCell},
     exceptions::{PyTypeError, PyValueError},
     prelude::*,
-    types::{PyList, PyUnicode},
+    types::{PyBytes, PyList, PyUnicode},
 };
 
 /// Search for multiple pattern strings against a single haystack string.
@@ -408,16 +408,22 @@ impl PyBytesAhoCorasick {
         haystack: &PyAny,
         overlapping: bool,
     ) -> PyResult<Vec<(u64, usize, usize)>> {
-        let haystack = PyBufferBytes::try_from(haystack)?;
-        let matches = get_matches(&self_.ac_impl, haystack.as_ref(), overlapping)?;
-
-        // Note: we must collect here and not release the GIL or return an iterator
-        // from this function due to the safety caveat in the implementation of
-        // AsRef<[u8]> for PyBufferBytes, which is relevant here since the matches
-        // iterator is holding an AsRef reference on the haystack.
-        Ok(matches
-            .map(|m| (m.pattern().as_u64(), m.start(), m.end()))
-            .collect())
+        let haystack_buffer = PyBufferBytes::try_from(haystack)?;
+        let matches = get_matches(&self_.ac_impl, haystack_buffer.as_ref(), overlapping)?
+            .map(|m| (m.pattern().as_u64(), m.start(), m.end()));
+
+        if !haystack.is_instance_of::<PyBytes>() {
+            // Note: we must collect here and not release the GIL or return an iterator
+            // from this function due to the safety caveat in the implementation of
+            // AsRef<[u8]> for PyBufferBytes, which is relevant here since the matches
+            // iterator is holding an AsRef reference to the haystack.
+            Ok(matches.collect())
+        } else {
+            // However, if the haystack is a PyBytes, it's guaranteed to be immutable,
+            // so the safety caveat doesn't apply, and we can safely release the GIL
+            // while the matches iterator is holding a reference to the haystack.
+            haystack.py().allow_threads(|| Ok(matches.collect()))
+        }
     }
 }
 

tests/test_ac_bytes.py

@@ -71,6 +71,35 @@ def test_different_byte_objects_matching(
     assert [haystack[s:e] for (_, s, e) in index_matches] == expected
 
 
+@pytest.mark.parametrize(
+    "implementation",
+    [
+        None,
+        Implementation.NoncontiguousNFA,
+        Implementation.ContiguousNFA,
+        Implementation.DFA,
+    ],
+)
+@pytest.mark.parametrize("haystack_type", [bytes, bytearray, memoryview])
+def test_different_byte_haystacks_matching(
+    implementation: Optional[Implementation],
+    haystack_type: type[bytes | bytearray | memoryview],
+) -> None:
+    """
+    find_matches_as_indexes() returns matching patterns in the given byte string.
+    """
+    haystack = haystack_type(b"hello, world, hello again")
+    patterns = [b"hello", b"world"]
+    ac = BytesAhoCorasick(patterns, implementation=implementation)
+
+    expected = [b"hello", b"world", b"hello"]
+
+    # find_matches_as_indexes()
+    index_matches = ac.find_matches_as_indexes(haystack)
+    assert [patterns[i] for (i, _, _) in index_matches] == expected
+    assert [haystack[s:e] for (_, s, e) in index_matches] == expected
+
+
 def test_iterator_of_patterns() -> None:
     """
     It's possible to construct ``BytesAhoCorasick()`` with an iterator.