Conversation
| fn next(zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<PyIterReturn> { | ||
| loop { | ||
| let pos = zelf.cur_idx.load(); | ||
| if pos >= zelf.iterables.len() { | ||
| break; | ||
| } | ||
| let cur_iter = if zelf.cached_iter.read().is_none() { | ||
| // We need to call "get_iter" outside of the lock. | ||
| let iter = zelf.iterables[pos].clone().get_iter(vm)?; | ||
| *zelf.cached_iter.write() = Some(iter.clone()); | ||
| iter | ||
| } else if let Some(cached_iter) = (*zelf.cached_iter.read()).clone() { | ||
| cached_iter | ||
| } else { | ||
| // Someone changed cached iter to None since we checked. | ||
| continue; | ||
| }; | ||
|
|
||
| // We need to call "next" outside of the lock. | ||
| match cur_iter.next(vm) { | ||
| Ok(PyIterReturn::Return(ok)) => return Ok(PyIterReturn::Return(ok)), | ||
| Ok(PyIterReturn::StopIteration(_)) => { | ||
| zelf.cur_idx.fetch_add(1); | ||
| *zelf.cached_iter.write() = None; | ||
| } | ||
| Err(err) => { | ||
| return Err(err); | ||
| let next = || { | ||
| let source = zelf.source.read().clone(); | ||
| match source { | ||
| None => { | ||
| return Ok(PyIterReturn::StopIteration(None)); | ||
| } | ||
| Some(source) => loop { | ||
| let active = zelf.active.read().clone(); | ||
| match active { | ||
| None => match source.next(vm) { | ||
| Ok(PyIterReturn::Return(ok)) => { | ||
| *zelf.active.write() = Some(ok.get_iter(vm)?); | ||
| } | ||
| Ok(PyIterReturn::StopIteration(_)) => { | ||
| return Ok(PyIterReturn::StopIteration(None)); | ||
| } | ||
| Err(err) => { | ||
| return Err(err); | ||
| } | ||
| }, | ||
| Some(active) => match active.next(vm) { | ||
| Ok(PyIterReturn::Return(ok)) => { | ||
| return Ok(PyIterReturn::Return(ok)); | ||
| } | ||
| Ok(PyIterReturn::StopIteration(_)) => { | ||
| *zelf.active.write() = None; | ||
| } | ||
| Err(err) => { | ||
| return Err(err); | ||
| } | ||
| }, | ||
| } | ||
| }, | ||
| } |
There was a problem hiding this comment.
📎 "It looks like you're matching variants of an Option. Would you like help?" 📎
if let Some(source) = zelf.source.read().clone() {
loop {
if let Some(active) = zelf.active.read().clone() {
match active.next(vm) {
Ok(PyIterReturn::Return(ok)) => {
return Ok(PyIterReturn::Return(ok));
}
Ok(PyIterReturn::StopIteration(_)) => {
*zelf.active.write() = None;
}
Err(err) => {
return Err(err);
}
}
} else {
match source.next(vm) {
Ok(PyIterReturn::Return(ok)) => {
*zelf.active.write() = Some(ok.get_iter(vm)?);
}
Ok(PyIterReturn::StopIteration(_)) => {
return Ok(PyIterReturn::StopIteration(None));
}
Err(err) => {
return Err(err);
}
}
}
}
} else {
Ok(PyIterReturn::StopIteration(None))
}
youknowone
left a comment
youknowone
left a comment
There was a problem hiding this comment.
Thank you! The changes looks good.
Do you have more works to do? I wonder what made this marked as draft.
vm/src/stdlib/itertools.rs
Outdated
| match source { | ||
| None => { | ||
| return Ok(PyIterReturn::StopIteration(None)); |
There was a problem hiding this comment.
| match source { | |
| None => { | |
| return Ok(PyIterReturn::StopIteration(None)); | |
| let source = if let Some(source) = source { | |
| source | |
| } else { | |
| return Ok(PyIterReturn::StopIteration(None)) | |
| }; |
I have different suggestion to @fanninpm's one. Let's keep indent depth shallower
There was a problem hiding this comment.
@youknowone that suggestion has a few syntax errors in lines 68 and 71.
EDIT: I stand corrected regarding line 68.
|
Thanks for the suggestions. I tried to incorporate them and I tried to reduce the nesting. I also removed the closure which I didn't like that much. Let me know what you think. If you have more suggestions, I'd be happy to hear them.
The main reason was the clippy test which failed. |
|
It fixed the ridiculously slow running of |
3 participants
Fix #3787