Skip to content

RFC: Simplify API w.r.t. string types and parallelism #455

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

adamreichold
Copy link

This drops the parallel Rust entry points as they offer little over direct usage of Rayon while forcing a copy and requiring the corpus to be available as a slice. The corresponding Python entry points are kept for convenience, but are implemented directly using Rayon instead.

Further, all string types are standardized on &str thereby avoiding unnecessary allocations when the data is already available in string form. Furthermore, the Python entry points can directly borrow from Python-managed string data via PyO3's PyBackedStr.

This is obviously not backwards-compatible on the Rust side but I think it is a simpler yet more efficient API to present there and hence wanted to at least propose this even though the difference is probably not worth a major version bump.

This drops the parallel Rust entry points as they offer little over direct usage
of Rayon while forcing a copy and requiring the corpus to be available as a
slice. The corresponding Python entry points are kept for convenience, but are
implemented directly using Rayon instead.

Further, all string types are standardized on `&str` thereby avoiding
unnecessary allocations when the data is already available in string form.
Furthermore, the Python entry points can directly borrow from Python-managed
string data via PyO3's `PyBackedStr`.

This is obviously not backwards-compatible on the Rust side but I think it is a
simpler yet more efficient API to present there and hence wanted to at least
propose this even though the difference is probably not worth a major version
bump.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant