From b1207ab39e9ba085b9b1624f2f01829904ffd08f Mon Sep 17 00:00:00 2001 From: fermian <78496870+fermian@users.noreply.github.com> Date: Mon, 12 Apr 2021 22:32:52 +0700 Subject: [PATCH 1/6] Update test_collections.py --- Lib/test/test_collections.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/test/test_collections.py b/Lib/test/test_collections.py index 12eeb0527..f560e9a33 100644 --- a/Lib/test/test_collections.py +++ b/Lib/test/test_collections.py @@ -1974,8 +1974,6 @@ class TestCounter(unittest.TestCase): self.assertRaises(TypeError, Counter, (), ()) self.assertRaises(TypeError, Counter.__init__) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_order_preservation(self): # Input order dictates items() order self.assertEqual(list(Counter('abracadabra').items()), From 4e9f9ff129c68b252742e2518a440914fbeef3f8 Mon Sep 17 00:00:00 2001 From: eldpswp99 Date: Mon, 16 Aug 2021 18:46:49 +0900 Subject: [PATCH 2/6] change Dict Entries type Vec to Vec> --- vm/src/dictdatatype.rs | 86 +++++++++++++++++++++++++++--------------- 1 file changed, 55 insertions(+), 31 deletions(-) diff --git a/vm/src/dictdatatype.rs b/vm/src/dictdatatype.rs index 925193f83..23e9f6e3c 100644 --- a/vm/src/dictdatatype.rs +++ b/vm/src/dictdatatype.rs @@ -25,6 +25,7 @@ type EntryIndex = usize; pub struct Dict { inner: PyRwLock>, } + impl fmt::Debug for Dict { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("Debug").finish() @@ -37,10 +38,12 @@ enum IndexEntry { Free, Index(usize), } + impl IndexEntry { const FREE: i64 = -1; const DUMMY: i64 = -2; } + impl From for IndexEntry { fn from(idx: i64) -> Self { match idx { @@ -50,6 +53,7 @@ impl From for IndexEntry { } } } + impl From for i64 { fn from(idx: IndexEntry) -> Self { match idx { @@ -65,7 +69,7 @@ struct DictInner { used: usize, filled: usize, indices: Vec, - entries: Vec>, + entries: Vec>>, } impl Clone for Dict { @@ -143,15 +147,19 @@ impl DictInner { self.indices = vec![IndexEntry::FREE; new_size]; let mask = (new_size - 1) as i64; for (entry_idx, entry) in self.entries.iter_mut().enumerate() { - let mut idxs = GenIndexes::new(entry.hash, mask); - loop { - let index_index = idxs.next(); - let idx = &mut self.indices[index_index]; - if *idx == IndexEntry::FREE { - *idx = entry_idx as i64; - entry.index = index_index; - break; + if let Some(entry) = entry { + let mut idxs = GenIndexes::new(entry.hash, mask); + loop { + let index_index = idxs.next(); + let idx = &mut self.indices[index_index]; + if *idx == IndexEntry::FREE { + *idx = entry_idx as i64; + entry.index = index_index; + break; + } } + } else { + //removed entry } } self.filled = self.used; @@ -172,7 +180,7 @@ impl DictInner { index, }; let entry_index = self.entries.len(); - self.entries.push(entry); + self.entries.push(Some(entry)); self.indices[index] = entry_index as i64; self.used += 1; if let IndexEntry::Free = index_entry { @@ -213,8 +221,8 @@ impl Dict { /// Store a key pub fn insert(&self, vm: &VirtualMachine, key: K, value: T) -> PyResult<()> - where - K: DictKey, + where + K: DictKey, { let hash = key.key_hash(vm)?; let _removed = loop { @@ -223,6 +231,7 @@ impl Dict { let mut inner = self.write(); // Update existing key if let Some(entry) = inner.entries.get_mut(index) { + let entry = entry.as_mut().unwrap(); if entry.index == index_index { let removed = std::mem::replace(&mut entry.value, value); // defer dec RC @@ -266,6 +275,7 @@ impl Dict { if let IndexEntry::Index(index) = entry { let inner = self.read(); if let Some(entry) = inner.entries.get(index) { + let entry = entry.as_ref().unwrap(); if entry.index == index_index { break Some(entry.value.clone()); } else { @@ -310,8 +320,8 @@ impl Dict { /// Delete a key pub fn delete(&self, vm: &VirtualMachine, key: K) -> PyResult<()> - where - K: DictKey, + where + K: DictKey, { if self.delete_if_exists(vm, &key)? { Ok(()) @@ -321,8 +331,8 @@ impl Dict { } pub fn delete_if_exists(&self, vm: &VirtualMachine, key: &K) -> PyResult - where - K: DictKey, + where + K: DictKey, { let hash = key.key_hash(vm)?; let deleted = loop { @@ -366,9 +376,9 @@ impl Dict { } pub fn setdefault(&self, vm: &VirtualMachine, key: K, default: F) -> PyResult - where - K: DictKey, - F: FnOnce() -> T, + where + K: DictKey, + F: FnOnce() -> T, { let hash = key.key_hash(vm)?; let res = loop { @@ -377,6 +387,7 @@ impl Dict { if let IndexEntry::Index(index) = entry { let inner = self.read(); if let Some(entry) = inner.entries.get(index) { + let entry = entry.as_ref().unwrap(); if entry.index == index_index { break entry.value.clone(); } else { @@ -408,9 +419,9 @@ impl Dict { key: K, default: F, ) -> PyResult<(PyObjectRef, T)> - where - K: DictKey, - F: FnOnce() -> T, + where + K: DictKey, + F: FnOnce() -> T, { let hash = key.key_hash(vm)?; let res = loop { @@ -419,6 +430,7 @@ impl Dict { if let IndexEntry::Index(index) = entry { let inner = self.read(); if let Some(entry) = inner.entries.get(index) { + let entry = entry.as_ref().unwrap(); if entry.index == index_index { break (entry.key.clone(), entry.value.clone()); } else { @@ -453,10 +465,18 @@ impl Dict { } pub fn next_entry(&self, position: &mut EntryIndex) -> Option<(PyObjectRef, T)> { - self.read().entries.get(*position).map(|entry| { - *position += 1; - (entry.key.clone(), entry.value.clone()) - }) + let inner = self.read(); + loop { + let entry = inner.entries.get(*position); + if let Some(entry) = entry { + *position += 1; + if let Some(entry) = entry { + break Some((entry.key.clone(), entry.value.clone())); + } + } else { + break None; + } + } } pub fn len_from_entry_index(&self, position: EntryIndex) -> usize { @@ -469,7 +489,9 @@ impl Dict { } pub fn keys(&self) -> Vec { - self.read().entries.iter().map(|v| v.key.clone()).collect() + self.read().entries.iter() + .filter(|v| v.is_some()) + .map(|v| v.as_ref().unwrap().key.clone()).collect() } /// Lookup the index for the given key. @@ -505,7 +527,7 @@ impl Dict { return Ok(idxs); } IndexEntry::Index(i) => { - let entry = &inner.entries[i]; + let entry = &inner.entries[i].as_ref().unwrap(); let ret = (IndexEntry::Index(i), index_index); if key.key_is(&entry.key) { break 'outer ret; @@ -540,7 +562,7 @@ impl Dict { return Ok(None); }; let mut inner = self.write(); - if matches!(inner.entries.get(entry_index), Some(entry) if entry.index == index_index) { + if matches!(inner.entries.get(entry_index), Some(entry) if entry.as_ref().unwrap().index == index_index) { // all good } else { // The dict was changed since we did lookup. Let's try again. @@ -551,12 +573,12 @@ impl Dict { let removed = if entry_index == inner.used { inner.entries.pop().unwrap() } else { - let last_index = inner.entries.last().unwrap().index; + let last_index = inner.entries.last().unwrap().as_ref().unwrap().index; let removed = inner.entries.swap_remove(entry_index); inner.indices[last_index] = entry_index as i64; removed }; - Ok(Some(removed)) + Ok(Some(removed.unwrap())) } /// Retrieve and delete a key @@ -576,6 +598,7 @@ impl Dict { pub fn pop_back(&self) -> Option<(PyObjectRef, T)> { let mut inner = self.write(); inner.entries.pop().map(|entry| { + let entry = entry.unwrap(); inner.used -= 1; inner.indices[entry.index] = IndexEntry::DUMMY; (entry.key, entry.value) @@ -638,6 +661,7 @@ impl DictKey for PyStrRef { } } } + impl DictKey for PyRefExact { fn key_hash(&self, vm: &VirtualMachine) -> PyResult { (**self).key_hash(vm) From ce99057c8103e19dee486fca212668e8d07e5b61 Mon Sep 17 00:00:00 2001 From: eldpswp99 Date: Tue, 17 Aug 2021 20:41:37 +0900 Subject: [PATCH 3/6] make dict delete remains order --- Lib/test/test_dict.py | 6 ---- vm/src/builtins/dict.rs | 2 +- vm/src/builtins/set.rs | 2 +- vm/src/dictdatatype.rs | 78 +++++++++++++++++++++++++---------------- 4 files changed, 49 insertions(+), 39 deletions(-) diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index ce449f7f9..77a5065e6 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -474,8 +474,6 @@ class DictTest(unittest.TestCase): for i in d: d[i+1] = 1 - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_mutating_iteration_delete(self): # change dict content during iteration d = {} @@ -485,8 +483,6 @@ class DictTest(unittest.TestCase): del d[0] d[0] = 0 - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_mutating_iteration_delete_over_values(self): # change dict content during iteration d = {} @@ -496,8 +492,6 @@ class DictTest(unittest.TestCase): del d[0] d[0] = 0 - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_mutating_iteration_delete_over_items(self): # change dict content during iteration d = {} diff --git a/vm/src/builtins/dict.rs b/vm/src/builtins/dict.rs index cfbdb5394..f25c37671 100644 --- a/vm/src/builtins/dict.rs +++ b/vm/src/builtins/dict.rs @@ -360,7 +360,7 @@ impl PyDict { #[pymethod] fn popitem(&self, vm: &VirtualMachine) -> PyResult { - if let Some((key, value)) = self.entries.pop_back() { + if let Some((key, value)) = self.entries.pop_back(vm) { Ok(vm.ctx.new_tuple(vec![key, value])) } else { let err_msg = vm.ctx.new_str("popitem(): dictionary is empty"); diff --git a/vm/src/builtins/set.rs b/vm/src/builtins/set.rs index 6c5cacba6..20f3972ea 100644 --- a/vm/src/builtins/set.rs +++ b/vm/src/builtins/set.rs @@ -226,7 +226,7 @@ impl PySetInner { fn pop(&self, vm: &VirtualMachine) -> PyResult { // TODO: should be pop_front, but that requires rearranging every index - if let Some((key, _)) = self.content.pop_back() { + if let Some((key, _)) = self.content.pop_back(vm) { Ok(key) } else { let err_msg = vm.ctx.new_str("pop from an empty set"); diff --git a/vm/src/dictdatatype.rs b/vm/src/dictdatatype.rs index 23e9f6e3c..2fe119df3 100644 --- a/vm/src/dictdatatype.rs +++ b/vm/src/dictdatatype.rs @@ -221,8 +221,8 @@ impl Dict { /// Store a key pub fn insert(&self, vm: &VirtualMachine, key: K, value: T) -> PyResult<()> - where - K: DictKey, + where + K: DictKey, { let hash = key.key_hash(vm)?; let _removed = loop { @@ -320,8 +320,8 @@ impl Dict { /// Delete a key pub fn delete(&self, vm: &VirtualMachine, key: K) -> PyResult<()> - where - K: DictKey, + where + K: DictKey, { if self.delete_if_exists(vm, &key)? { Ok(()) @@ -331,8 +331,8 @@ impl Dict { } pub fn delete_if_exists(&self, vm: &VirtualMachine, key: &K) -> PyResult - where - K: DictKey, + where + K: DictKey, { let hash = key.key_hash(vm)?; let deleted = loop { @@ -376,9 +376,9 @@ impl Dict { } pub fn setdefault(&self, vm: &VirtualMachine, key: K, default: F) -> PyResult - where - K: DictKey, - F: FnOnce() -> T, + where + K: DictKey, + F: FnOnce() -> T, { let hash = key.key_hash(vm)?; let res = loop { @@ -419,9 +419,9 @@ impl Dict { key: K, default: F, ) -> PyResult<(PyObjectRef, T)> - where - K: DictKey, - F: FnOnce() -> T, + where + K: DictKey, + F: FnOnce() -> T, { let hash = key.key_hash(vm)?; let res = loop { @@ -489,9 +489,12 @@ impl Dict { } pub fn keys(&self) -> Vec { - self.read().entries.iter() + self.read() + .entries + .iter() .filter(|v| v.is_some()) - .map(|v| v.as_ref().unwrap().key.clone()).collect() + .map(|v| v.as_ref().unwrap().key.clone()) + .collect() } /// Lookup the index for the given key. @@ -562,7 +565,8 @@ impl Dict { return Ok(None); }; let mut inner = self.write(); - if matches!(inner.entries.get(entry_index), Some(entry) if entry.as_ref().unwrap().index == index_index) { + if matches!(inner.entries.get(entry_index), Some(entry) if entry.as_ref().unwrap().index == index_index) + { // all good } else { // The dict was changed since we did lookup. Let's try again. @@ -570,14 +574,9 @@ impl Dict { }; inner.indices[index_index] = IndexEntry::DUMMY; inner.used -= 1; - let removed = if entry_index == inner.used { - inner.entries.pop().unwrap() - } else { - let last_index = inner.entries.last().unwrap().as_ref().unwrap().index; - let removed = inner.entries.swap_remove(entry_index); - inner.indices[last_index] = entry_index as i64; - removed - }; + let removed = inner.entries[entry_index].clone(); + inner.entries[entry_index] = None; + Ok(Some(removed.unwrap())) } @@ -595,14 +594,31 @@ impl Dict { Ok(removed) } - pub fn pop_back(&self) -> Option<(PyObjectRef, T)> { - let mut inner = self.write(); - inner.entries.pop().map(|entry| { - let entry = entry.unwrap(); - inner.used -= 1; - inner.indices[entry.index] = IndexEntry::DUMMY; - (entry.key, entry.value) - }) + pub fn pop_back(&self, vm: &VirtualMachine) -> Option<(PyObjectRef, T)> { + let inner = self.read(); + let mut idx: i64 = inner.size().entries_size as i64 - 1; + + while idx >= 0 && inner.entries.get(idx as usize).unwrap().is_none() { + idx -= 1; + } + + if idx < 0 { + None + } else { + let idx = idx as usize; + let entry = inner.entries.get(idx).unwrap().as_ref().unwrap().clone(); + + let removed_key = entry.key; + let removed_item = entry.value; + if let Ok(lookup) = self.lookup(vm, &removed_key, entry.hash, Some(inner)) { + match self.pop_inner(lookup) { + Ok(_) => Some((removed_key, removed_item)), + Err(_) => None, + } + } else { + None + } + } } pub fn sizeof(&self) -> usize { From fca7b4174427cdb97011d97c86a65cd7c733fda7 Mon Sep 17 00:00:00 2001 From: eldpswp99 Date: Tue, 17 Aug 2021 21:24:06 +0900 Subject: [PATCH 4/6] optimize popitem using nentries which implies next insertion index --- vm/src/dictdatatype.rs | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/vm/src/dictdatatype.rs b/vm/src/dictdatatype.rs index 2fe119df3..6bb7ff307 100644 --- a/vm/src/dictdatatype.rs +++ b/vm/src/dictdatatype.rs @@ -70,6 +70,8 @@ struct DictInner { filled: usize, indices: Vec, entries: Vec>>, + // index to new inserted element should be in entries + nentries: usize, } impl Clone for Dict { @@ -88,6 +90,7 @@ impl Default for Dict { filled: 0, indices: vec![IndexEntry::FREE; 8], entries: Vec::new(), + nentries: 0, }), } } @@ -107,6 +110,7 @@ pub struct DictSize { entries_size: usize, used: usize, filled: usize, + nentires: usize, } struct GenIndexes { @@ -163,6 +167,7 @@ impl DictInner { } } self.filled = self.used; + self.nentries = self.entries.len(); } fn unchecked_push( @@ -179,10 +184,15 @@ impl DictInner { value, index, }; - let entry_index = self.entries.len(); - self.entries.push(Some(entry)); + let entry_index = self.nentries; + if self.entries.len() == entry_index { + self.entries.push(Some(entry)); + } else { + self.entries[entry_index] = Some(entry); + } self.indices[index] = entry_index as i64; self.used += 1; + self.nentries += 1; if let IndexEntry::Free = index_entry { self.filled += 1; if let Some(new_size) = self.should_resize() { @@ -197,6 +207,7 @@ impl DictInner { entries_size: self.entries.len(), used: self.used, filled: self.filled, + nentires: self.nentries, } } @@ -313,6 +324,7 @@ impl Dict { inner.indices.resize(8, IndexEntry::FREE); inner.used = 0; inner.filled = 0; + inner.nentries = 0; // defer dec rc std::mem::take(&mut inner.entries) }; @@ -596,7 +608,7 @@ impl Dict { pub fn pop_back(&self, vm: &VirtualMachine) -> Option<(PyObjectRef, T)> { let inner = self.read(); - let mut idx: i64 = inner.size().entries_size as i64 - 1; + let mut idx: i64 = inner.nentries as i64 - 1; while idx >= 0 && inner.entries.get(idx as usize).unwrap().is_none() { idx -= 1; @@ -612,7 +624,11 @@ impl Dict { let removed_item = entry.value; if let Ok(lookup) = self.lookup(vm, &removed_key, entry.hash, Some(inner)) { match self.pop_inner(lookup) { - Ok(_) => Some((removed_key, removed_item)), + Ok(_) => { + let mut inner = self.write(); + inner.nentries = idx as usize; + Some((removed_key, removed_item)) + } Err(_) => None, } } else { From b49a1e9f5ed03ee16d6125987497143582d38609 Mon Sep 17 00:00:00 2001 From: eldpswp99 Date: Tue, 17 Aug 2021 22:50:32 +0900 Subject: [PATCH 5/6] remove test_ordered_dict delitem expected failure to passed test --- Lib/test/test_ordered_dict.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/test_ordered_dict.py b/Lib/test/test_ordered_dict.py index 8e64534e5..f52a6acc3 100644 --- a/Lib/test/test_ordered_dict.py +++ b/Lib/test/test_ordered_dict.py @@ -699,7 +699,6 @@ del method # TODO: RUSTPYTHON import functools setattr(CPythonBuiltinDictTests, "test_delitem", functools.partialmethod(OrderedDictTests.test_delitem)) -CPythonBuiltinDictTests.test_delitem = unittest.expectedFailure(CPythonBuiltinDictTests.test_delitem) @unittest.skipUnless(c_coll, 'requires the C version of the collections module') class CPythonOrderedDictTests(OrderedDictTests, unittest.TestCase): From 564eb2991e194e2d85abf259bfdd4efa9679bd5d Mon Sep 17 00:00:00 2001 From: eldpswp99 Date: Wed, 18 Aug 2021 02:28:58 +0900 Subject: [PATCH 6/6] resolving deadlock and redundant lookup, grammatic mistakes --- vm/src/builtins/dict.rs | 2 +- vm/src/builtins/set.rs | 2 +- vm/src/dictdatatype.rs | 89 +++++++++++++++++------------------------ 3 files changed, 39 insertions(+), 54 deletions(-) diff --git a/vm/src/builtins/dict.rs b/vm/src/builtins/dict.rs index f25c37671..cfbdb5394 100644 --- a/vm/src/builtins/dict.rs +++ b/vm/src/builtins/dict.rs @@ -360,7 +360,7 @@ impl PyDict { #[pymethod] fn popitem(&self, vm: &VirtualMachine) -> PyResult { - if let Some((key, value)) = self.entries.pop_back(vm) { + if let Some((key, value)) = self.entries.pop_back() { Ok(vm.ctx.new_tuple(vec![key, value])) } else { let err_msg = vm.ctx.new_str("popitem(): dictionary is empty"); diff --git a/vm/src/builtins/set.rs b/vm/src/builtins/set.rs index 20f3972ea..6c5cacba6 100644 --- a/vm/src/builtins/set.rs +++ b/vm/src/builtins/set.rs @@ -226,7 +226,7 @@ impl PySetInner { fn pop(&self, vm: &VirtualMachine) -> PyResult { // TODO: should be pop_front, but that requires rearranging every index - if let Some((key, _)) = self.content.pop_back(vm) { + if let Some((key, _)) = self.content.pop_back() { Ok(key) } else { let err_msg = vm.ctx.new_str("pop from an empty set"); diff --git a/vm/src/dictdatatype.rs b/vm/src/dictdatatype.rs index 6bb7ff307..f7bcc2e24 100644 --- a/vm/src/dictdatatype.rs +++ b/vm/src/dictdatatype.rs @@ -71,7 +71,7 @@ struct DictInner { indices: Vec, entries: Vec>>, // index to new inserted element should be in entries - nentries: usize, + next_new_entry_idx: usize, } impl Clone for Dict { @@ -90,7 +90,7 @@ impl Default for Dict { filled: 0, indices: vec![IndexEntry::FREE; 8], entries: Vec::new(), - nentries: 0, + next_new_entry_idx: 0, }), } } @@ -110,7 +110,6 @@ pub struct DictSize { entries_size: usize, used: usize, filled: usize, - nentires: usize, } struct GenIndexes { @@ -167,7 +166,7 @@ impl DictInner { } } self.filled = self.used; - self.nentries = self.entries.len(); + self.next_new_entry_idx = self.entries.len(); } fn unchecked_push( @@ -184,7 +183,7 @@ impl DictInner { value, index, }; - let entry_index = self.nentries; + let entry_index = self.next_new_entry_idx; if self.entries.len() == entry_index { self.entries.push(Some(entry)); } else { @@ -192,7 +191,7 @@ impl DictInner { } self.indices[index] = entry_index as i64; self.used += 1; - self.nentries += 1; + self.next_new_entry_idx += 1; if let IndexEntry::Free = index_entry { self.filled += 1; if let Some(new_size) = self.should_resize() { @@ -207,7 +206,6 @@ impl DictInner { entries_size: self.entries.len(), used: self.used, filled: self.filled, - nentires: self.nentries, } } @@ -242,7 +240,9 @@ impl Dict { let mut inner = self.write(); // Update existing key if let Some(entry) = inner.entries.get_mut(index) { - let entry = entry.as_mut().unwrap(); + let entry = entry + .as_mut() + .expect("The dict was changed since we did lookup."); if entry.index == index_index { let removed = std::mem::replace(&mut entry.value, value); // defer dec RC @@ -286,7 +286,7 @@ impl Dict { if let IndexEntry::Index(index) = entry { let inner = self.read(); if let Some(entry) = inner.entries.get(index) { - let entry = entry.as_ref().unwrap(); + let entry = extract_dict_entry(entry); if entry.index == index_index { break Some(entry.value.clone()); } else { @@ -324,7 +324,7 @@ impl Dict { inner.indices.resize(8, IndexEntry::FREE); inner.used = 0; inner.filled = 0; - inner.nentries = 0; + inner.next_new_entry_idx = 0; // defer dec rc std::mem::take(&mut inner.entries) }; @@ -399,7 +399,7 @@ impl Dict { if let IndexEntry::Index(index) = entry { let inner = self.read(); if let Some(entry) = inner.entries.get(index) { - let entry = entry.as_ref().unwrap(); + let entry = extract_dict_entry(entry); if entry.index == index_index { break entry.value.clone(); } else { @@ -442,7 +442,7 @@ impl Dict { if let IndexEntry::Index(index) = entry { let inner = self.read(); if let Some(entry) = inner.entries.get(index) { - let entry = entry.as_ref().unwrap(); + let entry = extract_dict_entry(entry); if entry.index == index_index { break (entry.key.clone(), entry.value.clone()); } else { @@ -479,14 +479,10 @@ impl Dict { pub fn next_entry(&self, position: &mut EntryIndex) -> Option<(PyObjectRef, T)> { let inner = self.read(); loop { - let entry = inner.entries.get(*position); + let entry = inner.entries.get(*position)?; + *position += 1; if let Some(entry) = entry { - *position += 1; - if let Some(entry) = entry { - break Some((entry.key.clone(), entry.value.clone())); - } - } else { - break None; + break Some((entry.key.clone(), entry.value.clone())); } } } @@ -504,8 +500,7 @@ impl Dict { self.read() .entries .iter() - .filter(|v| v.is_some()) - .map(|v| v.as_ref().unwrap().key.clone()) + .filter_map(|v| v.as_ref().map(|v| v.key.clone())) .collect() } @@ -577,7 +572,7 @@ impl Dict { return Ok(None); }; let mut inner = self.write(); - if matches!(inner.entries.get(entry_index), Some(entry) if entry.as_ref().unwrap().index == index_index) + if matches!(inner.entries.get(entry_index), Some(Some(entry)) if entry.index == index_index) { // all good } else { @@ -586,10 +581,8 @@ impl Dict { }; inner.indices[index_index] = IndexEntry::DUMMY; inner.used -= 1; - let removed = inner.entries[entry_index].clone(); - inner.entries[entry_index] = None; - - Ok(Some(removed.unwrap())) + let removed = std::mem::take(&mut inner.entries[entry_index]); + Ok(removed) } /// Retrieve and delete a key @@ -606,35 +599,21 @@ impl Dict { Ok(removed) } - pub fn pop_back(&self, vm: &VirtualMachine) -> Option<(PyObjectRef, T)> { - let inner = self.read(); - let mut idx: i64 = inner.nentries as i64 - 1; - - while idx >= 0 && inner.entries.get(idx as usize).unwrap().is_none() { - idx -= 1; + pub fn pop_back(&self) -> Option<(PyObjectRef, T)> { + let mut inner = self.write(); + let mut idx = inner.next_new_entry_idx.checked_sub(1)?; + while inner.entries.get(idx).unwrap().is_none() { + idx = idx.checked_sub(1)?; } - if idx < 0 { - None - } else { - let idx = idx as usize; - let entry = inner.entries.get(idx).unwrap().as_ref().unwrap().clone(); + let entry = std::mem::take(&mut inner.entries[idx]).unwrap(); + let removed_key = entry.key; + let removed_item = entry.value; - let removed_key = entry.key; - let removed_item = entry.value; - if let Ok(lookup) = self.lookup(vm, &removed_key, entry.hash, Some(inner)) { - match self.pop_inner(lookup) { - Ok(_) => { - let mut inner = self.write(); - inner.nentries = idx as usize; - Some((removed_key, removed_item)) - } - Err(_) => None, - } - } else { - None - } - } + inner.used -= 1; + inner.indices[entry.index] = IndexEntry::DUMMY; + inner.next_new_entry_idx = idx; + Some((removed_key, removed_item)) } pub fn sizeof(&self) -> usize { @@ -755,6 +734,12 @@ fn str_exact<'a>(obj: &'a PyObjectRef, vm: &VirtualMachine) -> Option<&'a PyStr> } } +fn extract_dict_entry(option_entry: &Option>) -> &DictEntry { + option_entry + .as_ref() + .expect("The dict was changed since we did lookup.") +} + #[cfg(test)] mod tests { use super::{Dict, DictKey};