Skip to content

Commit

Permalink
Use a python slice to represent a range to be fizzbuzzed (#46)
Browse files Browse the repository at this point in the history
Use new functionality from rust v3.0.0 plus build with `--release` optimisations giving up to 75x performance vs native python!
  • Loading branch information
MusicalNinjaDad authored Jun 3, 2024
1 parent a693fe2 commit e6f8268
Show file tree
Hide file tree
Showing 16 changed files with 395 additions and 36 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/coverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ jobs:
LLVM_PROFILE_FILE: tests-%p-%m.profraw
- name: analyse coverage
run: |
grcov . -s . --binary-path ./target/debug/ -t cobertura --branch --ignore-not-existing --output-path .
grcov . -s . --binary-path ./target/debug/ -t cobertura --branch --ignore-not-existing --excl-line GRCOV_EXCL_LINE --output-path .
pwd
ls -al
- name: Upload coverage reports to Codecov
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/python.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ on:
push:
paths:
- "**.py"
- "rust/fizzbuzzo3/src/*.rs"
- "pyproject.toml"
- ".github/workflows/python.yml"
pull_request:
Expand Down
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
# FizzBuzz Changelog

## Python 2.1.0

- Allow a `slice` to be passed to `fizzbuzzo3.fizzbuzz` - this provides a further 1.5x speed increase over passing a list with the same elements.
- Build rust with `--release` optimisations - another 4-7x speed increase.

## Rust 3.0.1

- Additional test case validating use case for stepped range, no code changes required.

## Rust 3.0.0

- **BREAKING CHANGE**: MultiFizzBuzz will consume the input.
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ members = [
"rust/*"
]

resolver = "2"
resolver = "2"
2 changes: 1 addition & 1 deletion __pyversion__
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2.0.1
2.1.0
107 changes: 103 additions & 4 deletions docs/dev-build.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ The [relevant section of the pyo3 book](https://pyo3.rs/latest/rust-from-python)
path = "rust/fizzbuzzo3/Cargo.toml"
binding = "PyO3"
features = ["pyo3/extension-module"] # IMPORTANT!!!
args = ["--release"] # 4-7x performance loss if you forget this
...
```

Expand All @@ -74,10 +75,15 @@ The [relevant section of the pyo3 book](https://pyo3.rs/latest/rust-from-python)

Background is available by combining the [pyo3 FAQ](https://pyo3.rs/latest/faq#i-cant-run-cargo-test-or-i-cant-build-in-a-cargo-workspace-im-having-linker-issues-like-symbol-not-found-or-undefined-reference-to-_pyexc_systemerror) and [manylinux specification](https://github.com/pypa/manylinux)

??? python "**`./pyproject.toml`** - full source"
```toml
--8<-- "./pyproject.toml"
```
!!! warning "Missing out on 4-7x performance gains"
Add `args = ["--release"]` to `./pyproject.toml` to ensure that your rust code is built with the right optimisations.

The default release profile is well documented in [The Cargo Book](https://doc.rust-lang.org/cargo/reference/profiles.html#release). I found a 4-7x performance boost when I enabled this!

??? python "**`./pyproject.toml`** - full source"
```toml
--8<-- "./pyproject.toml"
```

## Python virtual environment & build

Expand Down Expand Up @@ -205,6 +211,99 @@ Assuming part of the reason you are doing this is to provide a performance over
!!! tip "Check it makes sense"
Adding parallel processing doesn't always make sense as it adds overhead ramping and managing a threadpool. You will want to do some benchmarking to find the sweet-spot. Benchmarking and performance testing is a topic for itself, so I'll add a dedicated section ...

!!! pyo3 "Even more speed by passing a range (and implementing IntoPy and FromPyObject traits)"
The world obviously _needs_ the most performant fizzbuzz available! In an attempt to squeeze out even more speed I tried completely avoiding the need to build and pass a `list` and instead (ab)used a python `slice` to provide `start`, `stop`, and optional `step` values. This gave another 1.5x speed boost. Surprisingly most of that comes from passing the list to rust, not creating it or processing it:

!!! python "Timeit results"
```text
Rust: [3 calls of 10 runs fizzbuzzing up to 1000000]
[13.941677560000244, 12.671054376998654, 12.669853160998173]
Rust vector: [3 calls of 10 runs fizzbuzzing a list of numbers up to 1000000]
[5.104824486003054, 4.96210950999739, 4.903727466000419]
Rust vector, with python list overhead: [3 calls of 10 runs creating and fizzbuzzing a list of numbers up to 1000000]
[5.363066075999086, 5.316481181002018, 5.361383773997659]
Rust range: [3 calls of 10 runs fizzbuzzing a range of numbers up to 1000000]
[3.8294942710017494, 3.8227306799999496, 3.800879727001302]
```

!!! rust "Criterion bench results"
```text
multifizzbuzz_trait_from_vec_as_answer
time: [62.035 ms 63.960 ms 65.921 ms]
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mild

multifizzbuzz_trait_from_range_as_answer
time: [60.295 ms 62.228 ms 64.228 ms]
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mild
```
!!! rust "Excursion to follow ..."
This change was quite in depth, so expect an excursion later on the changes to the core `rust/fizzbuzz/src/lib.rs`...

!!! pyo3 "On the pyo3 side this involved the following in **`rust/fizzbuzzo3/src/lib.rs`**:"

1. Creating a `struct MySlice` to hold the start, stop and step values which:
1. Can be created from a python `slice`:
```rust
#[derive(FromPyObject)]
struct MySlice {
start: isize,
stop: isize,
step: Option<isize>,
}
```
1. Can be converted into a pyo3 `PySlice`:
```rust
impl IntoPy<Py<PyAny>> for MySlice {
fn into_py(self, py: Python<'_>) -> Py<PyAny> {
PySlice::new_bound(py, self.start, self.stop, self.step.unwrap_or(1)).into_py(py)
}
}
```
Note: There is [no rust standard type which pyo3 maps to a slice](https://pyo3.rs/latest/conversions/tables).
1. Parsing the slice to provide equivalent logic to python for negative steps:
```rust
fn py_fizzbuzz(num: FizzBuzzable) -> PyResult<String> {
match num {
...
FizzBuzzable::Slice(s) => match s.step {
None => Ok((s.start..s.stop).fizzbuzz().into()),
Some(1) => Ok((s.start..s.stop).fizzbuzz().into()),
Some(step) => match step {
1.. => Ok((s.start..s.stop)
.into_par_iter()
.step_by(step.try_into().unwrap())
.fizzbuzz()
.into()),

// ```python
// >>> foo[1:5:0]
// Traceback (most recent call last):
// File "<stdin>", line 1, in <module>
// ValueError: slice step cannot be zero
// ```
0 => Err(PyValueError::new_err("step cannot be zero")),

// ```python
// >>> foo=[0,1,2,3,4,5,6]
// >>> foo[6:0:-2]
// [6, 4, 2]
// ```
// Rust doesn't accept step < 0 or stop < start so need some trickery
..=-1 => Ok((s.start.neg()..s.stop.neg())
.into_par_iter()
.step_by(step.neg().try_into().unwrap())
.map(|x| x.neg())
.fizzbuzz()
.into()),
},
},
...
```
1. Quite a bit of extra testing ...

??? pyo3 "**`rust/fizzbuzzo3/src/lib.rs`** - full source"
```rust
--8<-- "rust/fizzbuzzo3/src/lib.rs"
Expand Down
14 changes: 7 additions & 7 deletions justfile
Original file line number Diff line number Diff line change
Expand Up @@ -59,25 +59,25 @@ cov:
RUSTFLAGS="-Cinstrument-coverage" cargo build
RUSTFLAGS="-Cinstrument-coverage" LLVM_PROFILE_FILE="tests-%p-%m.profraw" cargo test
-mkdir rustcov
grcov . -s . --binary-path ./target/debug/ -t html,lcov --branch --ignore-not-existing --output-path ./rustcov
grcov . -s . --binary-path ./target/debug/ -t html,lcov --branch --ignore-not-existing --excl-line GRCOV_EXCL_LINE --output-path ./rustcov
pytest --cov --cov-report html:pycov --cov-report term

# serve rust coverage results on localhost:8000 (doesn't run coverage analysis)
# serve rust coverage results on localhost:8002 (doesn't run coverage analysis)
rust-cov:
python -m http.server -d ./rustcov/html
python -m http.server -d ./rustcov/html 8002

# serve python coverage results on localhost:8000 (doesn't run coverage analysis)
# serve python coverage results on localhost:8003 (doesn't run coverage analysis)
py-cov:
python -m http.server -d ./pycov
python -m http.server -d ./pycov 8003

# serve python docs on localhost:8000
py-docs:
mkdocs serve

#build and serve rust API docs on localhost:8000
#build and serve rust API docs on localhost:8001
rust-docs:
cargo doc
python -m http.server -d target/doc
python -m http.server -d target/doc 8001

# build and test a wheel (a suitable venv must already by active!)
test-wheel: clean
Expand Down
6 changes: 4 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
path = "rust/fizzbuzzo3/Cargo.toml"
binding = "PyO3"
features = ["pyo3/extension-module"] # IMPORTANT!!!
args = ["--release"] # 4-7x performance loss if you forget this

[tool.cibuildwheel]
skip = [
Expand Down Expand Up @@ -86,11 +87,12 @@
# ===========================

[project.optional-dependencies]
stubs = ["pyo3-stubgen"]
lint = ["ruff"]
test = ["pytest", "pytest-doctest-mkdocstrings"]
cov = ["fizzbuzz[test]", "pytest-cov"]
doc = ["black", "mkdocs", "mkdocstrings[python]", "mkdocs-material"]
dev = ["fizzbuzz[lint,test,cov,doc]"]
doc = ["black", "mkdocs", "mkdocstrings[python]", "mkdocs-material", "fizzbuzz[stubs]"]
dev = ["fizzbuzz[stubs,lint,test,cov,doc]"]

[tool.pytest.ini_options]
xfail_strict = true
Expand Down
23 changes: 18 additions & 5 deletions python/fizzbuzz/fizzbuzzo3.pyi
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#ruff: noqa: PYI021
# flake8: noqa: PYI021
"""
An optimised rust version of fizzbuzz.
Expand All @@ -10,18 +10,19 @@ Usage:
```
"""

def fizzbuzz(n: int | list[int]) -> str:
def fizzbuzz(n: int | list[int] | slice) -> str:
"""
Returns the correct fizzbuzz answer for any number or list of numbers.
Returns the correct fizzbuzz answer for any number or list/range of numbers.
This is an optimised algorithm compiled in rust. Large lists will utilise multiple CPU cores for processing.
Passing a slice, to represent a range, is fastest.
Arguments:
n: the number(s) to fizzbuzz
Returns:
A `str` with the correct fizzbuzz answer(s).
In the case of a list of inputs the answers will be separated by `, `
In the case of a list or range of inputs the answers will be separated by `, `
Examples:
a single `int`:
Expand All @@ -38,4 +39,16 @@ def fizzbuzz(n: int | list[int]) -> str:
>>> fizzbuzz([1,3])
'1, fizz'
```
"""
a `slice` representing a range:
```
from fizzbuzz.fizzbuzzo3 import fizzbuzz
>>> fizzbuzz(slice(1,4,2))
'1, fizz'
>>> fizzbuzz(slice(1,4))
'1, 2, fizz'
>>> fizzbuzz(slice(4,1,-1))
'4, fizz, 2'
```
Note: Slices are inclusive on the left, exclusive on the right and can contain an optional step.
Negative steps require start > stop.
"""
2 changes: 1 addition & 1 deletion rust/fizzbuzz/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "fizzbuzz"
version = "3.0.0"
version = "3.0.1"
edition = "2021"

[lib]
Expand Down
17 changes: 17 additions & 0 deletions rust/fizzbuzz/tests/test_multifizzbuzz.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ mod vectors {
}

mod ranges {
use rayon::iter::{IndexedParallelIterator, IntoParallelIterator};

use super::*;

#[test]
Expand All @@ -34,4 +36,19 @@ mod ranges {
];
assert_eq!(answer, expected)
}

#[test]
fn test_range_with_step() {
let input = (0..16).into_par_iter().step_by(3);
let answer: Vec<String> = input.fizzbuzz().into();
let expected = vec![
"fizzbuzz".to_string(), // 0
"fizz".to_string(), // 3
"fizz".to_string(), // 6
"fizz".to_string(), // 9
"fizz".to_string(), // 12
"fizzbuzz".to_string(), //15
];
assert_eq!(answer, expected)
}
}
7 changes: 5 additions & 2 deletions rust/fizzbuzzo3/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "fizzbuzzo3"
version = "1.3.0"
version = "2.1.0" # Tracks python version
edition = "2021"

[lib]
Expand All @@ -9,8 +9,11 @@ path = "src/lib.rs"
crate-type = ["cdylib"] # cdylib required for python import, rlib required for rust tests.

[dependencies]
fizzbuzz = { path = "../fizzbuzz" }
pyo3 = "0.21.2"
rayon = "1.10.0"

[dev-dependencies]
pyo3-testing = "0.3.4"
fizzbuzz = { path = "../fizzbuzz" }

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
Loading

0 comments on commit e6f8268

Please sign in to comment.