-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: multi threaded pipeline #2213
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
base: feat/train_encode_split
Are you sure you want to change the base?
Changes from all commits
c6d306f
22033b3
61a82cd
f866644
addabaa
3765e0a
1c0f9e5
b644aba
8581dfa
8aec88b
e52925c
2f0e387
502347f
8596486
852c82a
6610330
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| 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 { | ||||||||||||||
| ptr: *const u8, | ||||||||||||||
| len: usize, | ||||||||||||||
|
Comment on lines
+19
to
+20
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't we use It seems to be doing exactly what you've re-implemented here, and provide safe APIs to get
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
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?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how does it assist? why does it matter?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. canceling return claimed and finished work in a wait?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
not sure I understand the question, but if the |
||||||||||||||
| #[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()) | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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))?)))), | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PyStringis necessarily a python string from python world 😄Not sure a comment is needed here, imo quite explicit what this is?