Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bindings/python/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions bindings/python/py_src/tokenizers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,10 @@ class SplitDelimiterBehavior(Enum):

from .tokenizers import (
AddedToken,
EncodeHandle,
Encoding,
NormalizedString,
PipelineTokenizer,
PreTokenizedString,
Regex,
Token,
Expand Down
5 changes: 5 additions & 0 deletions bindings/python/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ mod processors;
mod token;
mod tokenizer;
mod trainers;
mod pipeline;
mod utils;

use pyo3::prelude::*;
Expand Down Expand Up @@ -60,6 +61,10 @@ pub mod tokenizers {
#[pymodule_export]
pub use super::tokenizer::PyTokenizer;
#[pymodule_export]
pub use super::pipeline::PyPipelineTokenizer;
#[pymodule_export]
pub use super::pipeline::PyEncodeHandle;
#[pymodule_export]
pub use super::utils::PyNormalizedString;
#[pymodule_export]
pub use super::utils::PyPreTokenizedString;
Expand Down
172 changes: 172 additions & 0 deletions bindings/python/src/pipeline.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
//! Pipeline python bindings

use std::sync::Mutex;

use pyo3::exceptions;
use pyo3::prelude::*;
use pyo3::types::PyString;

use tk::pipeline::{EncodeHandle, Inputs, IntoInputs, PipelineToken, PipelineTokenizer};

use crate::error::ToPyResult;
use crate::tokenizer::PyTokenizer;

fn ids(tokens: Vec<PipelineToken>) -> Vec<u32> {
tokens.iter().map(|t| t.id).collect()
}

struct PyStrView {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
struct PyStrView {
// A pointer to a utf8-encoded PyString that comes/lives in python world.
struct PyStrView {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PyString is necessarily a python string from python world 😄

Not sure a comment is needed here, imo quite explicit what this is?

ptr: *const u8,
len: usize,
Comment on lines +19 to +20

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we use PyBackedString from pyo3 instead?

It seems to be doing exactly what you've re-implemented here, and provide safe APIs to get &str from the Python-owned object

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

/// Keep alive mechanism for zero-copy access to utf8 python strings
struct PyStrBatch {
/// the keep alive ref
_owners: Vec<Py<PyString>>,
/// (ptr, len)
views: Vec<PyStrView>,
}

// SAFETY: we only ever read from the underlying buffers,
// and the lifetime of the buffers is guaranteed by storing
// the Py<PyString> in _owners, which is kept alive as long as
// a reference is held, because Python refcount.
unsafe impl Send for PyStrBatch {}
unsafe impl Sync for PyStrBatch {}

impl PyStrBatch {
fn new(strings: Vec<Bound<'_, PyString>>) -> PyResult<Self> {
let mut owners = Vec::with_capacity(strings.len());
let mut views = Vec::with_capacity(strings.len());
for s in strings {
let view = s.to_str()?;
views.push(PyStrView {
ptr: view.as_ptr(),
len: view.len(),
});
owners.push(s.unbind());
}
Ok(Self {
_owners: owners,
views,
})
}
}

impl Inputs for PyStrBatch {
fn len(&self) -> usize {
self.views.len()
}

fn get(&self, i: usize) -> &str {
let (ptr, len) = (self.views[i].ptr, self.views[i].len);
// SAFETY: ptr is kept alive by PyStrBatch::_owners, and the underlying buffer is immutable UTF-8
unsafe { std::str::from_utf8_unchecked(std::slice::from_raw_parts(ptr, len)) }
Comment on lines +64 to +65

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// SAFETY: ptr is kept alive by PyStrBatch::_owners, and the underlying buffer is immutable UTF-8
unsafe { std::str::from_utf8_unchecked(std::slice::from_raw_parts(ptr, len)) }
// SAFETY: the pointer given by view.as_ptr() is non null even for empty str. The memory range is not freed as PyStrBatch::_owners protects it.
let slice =unsafe { std::slice::from_raw_parts(ptr, len)}
// SAFETY: the pointed string IS a utf8 string it does not require another validation (O(len)).
unsafe { std::str::from_utf8_unchecked(slice) };

I think its important to explain why we would go through unsafe when safe can work just as well + not hide to unsafe calls but split them?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Safe would require a clone here, when it's not necessary because python guarantees the buffer won't be gc-ed as long as we have a reference (this is what _owners is for).
Python also guarantees immutability of the buffer, so we can safely create a &str from it. Also the .to_str call we do in PyStrBatch::new guarantees it'll be utf8. So unsafe is needed here, because we're accessing the string's buffer via raw ptr mechanics (we have to if we want to avoid cloning the strings, which is not needed and thanks to this we're 0-copy from python input to rust, we won't be able to achieve something similar without unsafe), but we know the safety guarantees are upheld in this context.

            let view = s.to_str()?; // -> guarantees the string buffer's bytes are utf8, then cached and never mutated
            views.push(PyStrView {
                ptr: view.as_ptr(),
                len: view.len(),
            });
            owners.push(s.unbind());

}
}

impl IntoInputs for PyStrBatch {
type Inputs = PyStrBatch;
fn into_inputs(self) -> PyStrBatch {
self
}
}

Comment on lines +69 to +75

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as this is gonna change with the updates to support pairs of inputs, makes sense to have just a file for input?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

erm, not sure, I think we can extend this struct to support pairs of inputs rather than replacing it. Not sure what you mean by "file for input"

#[pyclass(module = "tokenizers", name = "PipelineTokenizer")]
pub struct PyPipelineTokenizer {
pipeline: PipelineTokenizer,
}

#[pymethods]
impl PyPipelineTokenizer {
#[staticmethod]
fn from_tokenizer(tokenizer: &PyTokenizer) -> PyResult<Self> {
let json = {
let guard = tokenizer.read_inner()?;
serde_json::to_string(&*guard)
.map_err(|e| exceptions::PyException::new_err(format!("{e}")))?
};
let tok: tk::Tokenizer = serde_json::from_str(&json)
.map_err(|e| exceptions::PyException::new_err(format!("{e}")))?;
let pipeline = PyResult::from(ToPyResult(PipelineTokenizer::try_from(&tok)))?;
Ok(Self { pipeline })
}

#[staticmethod]
fn from_file(path: &str) -> PyResult<Self> {
let tok = PyResult::from(ToPyResult(tk::Tokenizer::from_file(path)))?;
let pipeline = PyResult::from(ToPyResult(PipelineTokenizer::try_from(&tok)))?;
Ok(Self { pipeline })
}

/// Encode a batch of `str`, returning an [`PyEncodeHandle`] as soon as possible:
/// pool workers encode in the background while you hold the job, `wait()`
/// for everything (returned in input order), or iterate `(index, ids)` as
/// each input completes. Zero-copy: the job reads the Python strings' UTF-8
/// buffers in place.
fn encode_batch(
&self,
py: Python<'_>,
input: Vec<Bound<'_, PyString>>,
) -> PyResult<PyEncodeHandle> {
let batch = PyStrBatch::new(input)?;
// Below the cost gate the job is computed inline — release the GIL.
let job = py.detach(|| self.pipeline.encode(batch));
Ok(PyEncodeHandle {
job: Mutex::new(Some(job)),
})
}
}

/// An in-flight (or completed) batch encode. `wait()` blocks for all inputs and
/// returns their ids lists in input order; iterating yields `(index, ids)` for
/// each input as it completes (completion order, not input order — use `index`
/// to place it). The consuming thread *assists* the pool while it waits (all
/// with the GIL released). Dropping the job cancels unclaimed work.
Comment on lines +125 to +126

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how does it assist? why does it matter?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

canceling return claimed and finished work in a wait?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how does it assist? why does it matter?

impl Iterator for EncodeHandle {
    type Item = (usize, Result<Vec<PipelineToken>>);
    fn next(&mut self) -> Option<Self::Item> {
        match &mut self.inner {
            HandleInner::Ready(it) => it.next(),
            HandleInner::Streaming { core, state } => state.next_completed(core),
        }
    }
}

impl StreamState {
    fn next_completed(&mut self, core: &JobCore) -> Option<(usize, Result<Vec<PipelineToken>>)> {
        if self.next_k >= self.n {
            return None;
        }
        loop {
            let seq = core.completed_order[self.next_k].load(Ordering::Acquire);
            if seq != NOT_DONE {
                self.next_k += 1;
                return Some((seq, core.take_result(seq)));
            }
            // <------ HERE IS THE ASSIST LOGIC ------>
            if !self.assist_done {
                if SCRATCH.with(|st| core.run_one(&mut st.borrow_mut())) {
                    continue;
                }
                self.assist_done = true;
            }
            std::hint::spin_loop();
        }
    }
}

I've added a comment to show where the assist logic is.

Why:

  • sleep is out of the question, too unpredictable and adds too many syscalls
  • spin (loop { if has_result() { break; } }) burns a core for nothing, although it reacts fast
  • condvar/futex (park) is the only viable alternative imo, reacts in the µs, but if we do implement it over assist, it has the following downsides:
    • latency: encode can start straight away, even before the pool has initialised
    • no context switching, the thread is always alive, no need to pay for µs wake
    • no need for the workers to signal the main thread that results are ready
    • overall more complicated, needs quite some code to implement, whereas assist is simply "call the work function we already have each worker do"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

canceling return claimed and finished work in a wait?

not sure I understand the question, but if the EncodeHandle is dropped for wtv reason, the ongoing encode jobs are cancelled

#[pyclass(module = "tokenizers", name = "EncodeHandle")]
pub struct PyEncodeHandle {
job: Mutex<Option<EncodeHandle>>,
}

impl PyEncodeHandle {
fn take(&self) -> PyResult<EncodeHandle> {
self.job
.lock()
.unwrap_or_else(|e| e.into_inner())
.take()
.ok_or_else(|| exceptions::PyException::new_err("EncodeHandle already consumed"))
}
}

#[pymethods]
impl PyEncodeHandle {
/// Block until every input is encoded, returning ids lists in input order
fn wait(&self, py: Python<'_>) -> PyResult<Vec<Vec<u32>>> {
let job = self.take()?;
let res = py.detach(|| job.wait_for_completion());
let out = PyResult::from(ToPyResult(res))?;
Ok(out.into_iter().map(ids).collect())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we should be able to send data to python without allocating (maybe through numpy array?)

Probably as a follow-up

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes agree, will tackle later

}

fn __iter__(slf: PyRef<'_, Self>) -> PyRef<'_, Self> {
slf
}

/// Yield `(input index, ids)` for each input as it finishes — completion
/// order, not input order. `index` is the position in the batch passed to
/// `encode_batch` (always `0` for a single input).
fn __next__(&self, py: Python<'_>) -> PyResult<Option<(usize, Vec<u32>)>> {
let next = py.detach(|| {
self.job
.lock()
.unwrap_or_else(|e| e.into_inner())
.as_mut()
.and_then(|j| j.next())
});
match next {
None => Ok(None),
Some((seq, res)) => Ok(Some((seq, ids(PyResult::from(ToPyResult(res))?)))),
}
}
}
Loading