From 3d1885e165869c786c5f925b19c1b6c7e5c44ee3 Mon Sep 17 00:00:00 2001 From: bunnie Date: Thu, 27 Oct 2022 01:10:17 +0800 Subject: [PATCH] add test code results are slightly better on the first run (3700ms instead of 4900ms) but there seems to be really excessive allocations using the heap allocated serializer by rkyv. We really need to use the bufserializer I think for this to work. --- apps/vault/src/actions.rs | 45 +++++++++++++++++++++++++++++++++++++++ services/pddb/src/lib.rs | 4 ++-- services/pddb/src/main.rs | 26 +++++++++++++--------- 3 files changed, 63 insertions(+), 12 deletions(-) diff --git a/apps/vault/src/actions.rs b/apps/vault/src/actions.rs index 69d297071..54549663b 100644 --- a/apps/vault/src/actions.rs +++ b/apps/vault/src/actions.rs @@ -684,6 +684,50 @@ impl<'a> ActionManager<'a> { match self.mode_cache { VaultMode::Password => { let start = self.tt.elapsed_ms(); + let mut klen = 0; + match self.pddb.borrow().read_dict(VAULT_PASSWORD_DICT, None, Some(256 * 1024)) { + Ok(keys) => { + let mut oom_keys = 0; + for key in keys { + if let Some(data) = key.data { + if let Some(pw) = storage::PasswordRecord::try_from(data).ok() { + let extra = format!("{}; {}{}", + crate::ux::atime_to_str(pw.atime), + t!("vault.u2f.appinfo.authcount", xous::LANG), + pw.count, + ); + let desc = format!("{}/{}", pw.description, pw.username); + let li = ListItem { + name: desc, + extra, + dirty: true, + guid: key.name, + }; + klen += 1; + il.push(li); + } + } else { + oom_keys += 1; + } + } + if oom_keys != 0 { + log::warn!("Ran out of cache space handling password keys. {} keys are not loaded.", oom_keys); + self.report_err(&format!("Ran out of cache space handling passwords. {} passwords not loaded", oom_keys), None::); + } + } + Err(e) => { + match e.kind() { + ErrorKind::NotFound => { + // this is fine, it just means no passwords have been entered yet + }, + _ => { + log::error!("Error opening password dictionary"); + self.report_err("Error opening password dictionary", Some(e)) + } + } + }, + } + /* #[cfg(feature="vaultperf")] self.perfentry(&self.pm, PERFMETA_STARTBLOCK, 1, std::line!()); let keylist = match self.pddb.borrow().list_keys(VAULT_PASSWORD_DICT, None) { @@ -771,6 +815,7 @@ impl<'a> ActionManager<'a> { } #[cfg(feature="vaultperf")] self.perfentry(&self.pm, PERFMETA_ENDBLOCK, 2, std::line!()); + */ log::info!("readout took {} ms for {} elements", self.tt.elapsed_ms() - start, klen); } VaultMode::Fido => { diff --git a/services/pddb/src/lib.rs b/services/pddb/src/lib.rs index 01d7d671a..197b6a414 100644 --- a/services/pddb/src/lib.rs +++ b/services/pddb/src/lib.rs @@ -767,7 +767,7 @@ impl Pddb { /// Defaults to a size limit of up to 32k of bulk data returned, if it is not explicitly specified. pub fn read_dict(&self, dict: &str, basis: Option<&str>, size_limit: Option) -> Result> { // about the biggest we can move around in Precursor and not break heap. - const MAX_BUFLEN: usize = 128 * 1024; + const MAX_BUFLEN: usize = 32 * 1024; // compromise between memory zeroing time and latency to send a message const DEFAULT_LIMIT: usize = 32 * 1024; /* @@ -939,7 +939,7 @@ impl Pddb { if check_total_keys != check_key_index { log::error!("Number of keys read does not match expected value: {}, {}", check_total_keys, check_key_index); } - + xous::unmap_memory(msg_mem).unwrap(); Ok(ret) } diff --git a/services/pddb/src/main.rs b/services/pddb/src/main.rs index e7383a08a..c8bbc107d 100644 --- a/services/pddb/src/main.rs +++ b/services/pddb/src/main.rs @@ -1405,13 +1405,16 @@ fn wrapped_main() -> ! { } // Optimized bulk data read handler. See lib.rs for documentation. - // TO INVESTIGATE: probably serialization can be made a lot more efficient by - // using a `BufferSerializer` instead of a WriteSeriaizer. I think we can also - // stack multiple records into the buffer without having to re-allocate the serializer, - // but it's not clear to me how the deserialization would work in that case. The main - // problem of using the BufferSerializer is the interior mutability problem of re-using the - // allocated slice across multiple iterations of the loop, especially with the property where - // if the record doesn't fit we want to stash it elsewhere for use on the next iteration. + // Note: it would be tempting to use a `BufferSerializer` instead of a WriteSeriaizer. + // This would prevent the extra allocation and copy into the buffer. The problem is that + // the BufferSerializer can return you the `pos` of the archive, but it won't tell you + // the length of the data that's been archived. Thus, when you call `into_inner()` you get + // back the original buffer (which is great -- we could split that and pass it on to future + // iterations) but we don't know how much data was put in there. Maybe later versions of rkyv + // have a call for that, but the current version we are using doesn't seem to be written to + // allow for multiple rkyv objects to be packed into a single packet. I think the assumption + // is rather that you're writing rkyv objects into a single file and the files are then packed + // into disk by the filesystem. Opcode::DictBulkRead => { const BULKREAD_TIMEOUT_MS: u64 = 5000; let range = msg.body.memory_message_mut().unwrap(); @@ -1476,6 +1479,7 @@ fn wrapped_main() -> ! { }; bulkread_state = Some(state); } + let mut finished = false; // handle data packing into the structure if let Some(state) = &mut bulkread_state { // check that the token matches, if this isn't a first call to the function @@ -1493,7 +1497,6 @@ fn wrapped_main() -> ! { // now loop through and pack data into the slice let mut index = size_of::(); - let mut finished = false; loop { let (pos, sbuf) = if let Some((pos, sbuf)) = state.serialized.take() { (pos, sbuf) @@ -1568,9 +1571,8 @@ fn wrapped_main() -> ! { } }; // sbuf now contains some serialized data. will it fit? Need 2x`u32` as recordkeeeping overhead. - assert!(sbuf.len() == sbuf.as_slice().len(), "remove this assert if it doesn't fail on first run"); if sbuf.len() + size_of::() * 2 <= buf.len() - index { - log::info!("packing message of {}({})", sbuf.len(), pos); + log::debug!("packing message of {}({})", sbuf.len(), pos); // data can fit, copy it into the buffer. state.buf_starting_key_index += 1; header.len += 1; @@ -1587,6 +1589,7 @@ fn wrapped_main() -> ! { sbuf.as_slice() ); index += sbuf.len(); + drop(sbuf); } else { // log::info!("{} <= {}", sbuf.len() + size_of::() * 2, buf.len()); assert!( @@ -1613,6 +1616,9 @@ fn wrapped_main() -> ! { } else { log::warn!("This should be unreachable, the state should always be initialized by this point"); } + if finished { + bulkread_state = None; + } } Opcode::SeekKeyStd => {