Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: bidirectional references and cleanup #345

Draft
wants to merge 20 commits into
base: master
Choose a base branch
from
Draft

feat: bidirectional references and cleanup #345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
87ec8c3
start of work
QuantumExplorer Jul 28, 2024
5f4a53d
Merge remote-tracking branch 'origin/master' into feat/bidirectional_…
fominok Oct 22, 2024
088b149
make it at least compile
fominok Oct 22, 2024
b7730f1
wip
fominok Oct 24, 2024
49411e6
wip
fominok Oct 25, 2024
bdef719
wip
fominok Oct 25, 2024
15b3034
wip
fominok Oct 29, 2024
6af542b
remove redundant error
fominok Oct 29, 2024
e8385ae
wip
fominok Nov 1, 2024
4af11cf
wip
fominok Nov 1, 2024
ac4b627
wip
fominok Nov 4, 2024
740d97d
wip
fominok Nov 5, 2024
eb7b6b2
wip
fominok Nov 5, 2024
ea61c39
wip
fominok Nov 5, 2024
2f134ac
wip
fominok Nov 6, 2024
72f519f
wip
fominok Nov 6, 2024
cd47589
wip
fominok Nov 7, 2024
644efb3
wip
fominok Nov 7, 2024
be17531
wip
fominok Nov 7, 2024
0d92ceb
wip
fominok Nov 8, 2024
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
16 changes: 13 additions & 3 deletions costs/src/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,15 @@
pub fn cost_as_result(self) -> Result<OperationCost, E> {
self.value.map(|_| self.cost)
}

/// Call the provided function on success without altering result or cost.
pub fn for_ok(self, f: impl FnOnce(&T)) -> CostResult<T, E> {
if let Ok(x) = &self.value {
f(x)
}

self
}
}

impl<T, E> CostResult<Result<T, E>, E> {
Expand Down Expand Up @@ -170,8 +179,9 @@
/// 1. Early termination on error;
/// 2. Because of 1, `Result` is removed from the equation;
/// 3. `CostContext` is removed too because it is added to external cost
/// accumulator; 4. Early termination uses external cost accumulator so previous
/// costs won't be lost.
/// accumulator;

Check warning on line 182 in costs/src/context.rs

View workflow job for this annotation

GitHub Actions / clippy

doc list item without indentation 

warning: doc list item without indentation
   --> costs/src/context.rs:182:5
    |
182 | /// accumulator;
    |     ^
    |
    = help: if this is supposed to be its own paragraph, add a blank line
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#doc_lazy_continuation
    = note: `#[warn(clippy::doc_lazy_continuation)]` on by default
help: indent this line
    |
182 | ///    accumulator;
    |     +++
/// 4. Early termination uses external cost accumulator so previous costs won't
/// be lost.
#[macro_export]
macro_rules! cost_return_on_error {
( &mut $cost:ident, $($body:tt)+ ) => {
Expand All @@ -193,7 +203,7 @@
/// so no costs will be added except previously accumulated.
#[macro_export]
macro_rules! cost_return_on_error_no_add {
( &$cost:ident, $($body:tt)+ ) => {
( $cost:ident, $($body:tt)+ ) => {
{
use $crate::CostsExt;
let result = { $($body)+ };
Expand Down
1 change: 1 addition & 0 deletions grovedb-version/src/version/grovedb_versions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ pub struct GroveDBElementMethodVersions {
pub query_item: FeatureVersion,
pub basic_push: FeatureVersion,
pub serialize: FeatureVersion,
pub serialize_for_value_hash: FeatureVersion,
pub serialized_size: FeatureVersion,
pub deserialize: FeatureVersion,
}
Expand Down
1 change: 1 addition & 0 deletions grovedb-version/src/version/v1.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ pub const GROVE_V1: GroveVersion = GroveVersion {
query_item: 0,
basic_push: 0,
serialize: 0,
serialize_for_value_hash: 0,
serialized_size: 0,
deserialize: 0,
},
Expand Down
8 changes: 4 additions & 4 deletions grovedb/src/batch/estimated_costs/average_case_costs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@
let mut cost = OperationCost::default();

let layer_element_estimates = cost_return_on_error_no_add!(
&cost,
cost,
self.paths.get(path).ok_or_else(|| {
let paths = self
.paths
Expand All @@ -216,9 +216,9 @@
.estimated_to_be_empty();

// Then we have to get the tree
if self.cached_merks.get(path).is_none() {

Check warning on line 219 in grovedb/src/batch/estimated_costs/average_case_costs.rs

View workflow job for this annotation

GitHub Actions / clippy

unnecessary use of `get(path).is_none()` 

warning: unnecessary use of `get(path).is_none()`
   --> grovedb/src/batch/estimated_costs/average_case_costs.rs:219:30
    |
219 |         if self.cached_merks.get(path).is_none() {
    |            ------------------^^^^^^^^^^^^^^^^^^^
    |            |
    |            help: replace it with: `!self.cached_merks.contains_key(path)`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_get_then_check
    = note: `#[warn(clippy::unnecessary_get_then_check)]` on by default
let layer_info = cost_return_on_error_no_add!(
&cost,
cost,
self.paths.get(path).ok_or_else(|| {
let paths = self
.paths
Expand All @@ -233,7 +233,7 @@
})
);
cost_return_on_error_no_add!(
&cost,
cost,
GroveDb::add_average_case_get_merk_at_path::<RocksDbStorage>(
&mut cost,
path,
Expand Down Expand Up @@ -270,22 +270,22 @@
let base_path = KeyInfoPath(vec![]);
if let Some(estimated_layer_info) = self.paths.get(&base_path) {
// Then we have to get the tree
if !self.cached_merks.contains_key(&base_path) {
cost_return_on_error_no_add!(
&cost,
cost,
GroveDb::add_average_case_get_merk_at_path::<RocksDbStorage>(
&mut cost,
&base_path,
estimated_layer_info
.estimated_layer_count
.estimated_to_be_empty(),
estimated_layer_info.is_sum_tree,
grove_version
)
);
self.cached_merks
.insert(base_path, estimated_layer_info.is_sum_tree);
}

Check warning on line 288 in grovedb/src/batch/estimated_costs/average_case_costs.rs

View workflow job for this annotation

GitHub Actions / clippy

usage of `contains_key` followed by `insert` on a `HashMap` 

warning: usage of `contains_key` followed by `insert` on a `HashMap`
   --> grovedb/src/batch/estimated_costs/average_case_costs.rs:273:13
    |
273 | /             if !self.cached_merks.contains_key(&base_path) {
274 | |                 cost_return_on_error_no_add!(
275 | |                     cost,
276 | |                     GroveDb::add_average_case_get_merk_at_path::<RocksDbStorage>(
...   |
287 | |                     .insert(base_path, estimated_layer_info.is_sum_tree);
288 | |             }
    | |_____________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_entry
    = note: `#[warn(clippy::map_entry)]` on by default
help: try
    |
273 ~             if let std::collections::hash_map::Entry::Vacant(e) = self.cached_merks.entry(base_path) {
274 +                 cost_return_on_error_no_add!(
275 +                     cost,
276 +                     GroveDb::add_average_case_get_merk_at_path::<RocksDbStorage>(
277 +                         &mut cost,
278 +                         &base_path,
279 +                         estimated_layer_info
280 +                             .estimated_layer_count
281 +                             .estimated_to_be_empty(),
282 +                         estimated_layer_info.is_sum_tree,
283 +                         grove_version
284 +                     )
285 +                 );
286 +                 e.insert(estimated_layer_info.is_sum_tree);
287 +             }
    |
}
Ok(()).wrap_with_cost(cost)
}
Expand Down
6 changes: 3 additions & 3 deletions grovedb/src/batch/estimated_costs/worst_case_costs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ impl<G, SR> TreeCache<G, SR> for WorstCaseTreeCacheKnownPaths {
let mut cost = OperationCost::default();

let worst_case_layer_element_estimates = cost_return_on_error_no_add!(
&cost,
cost,
self.paths
.get(path)
.ok_or_else(|| Error::PathNotFoundInCacheForEstimatedCosts(format!(
Expand All @@ -204,7 +204,7 @@ impl<G, SR> TreeCache<G, SR> for WorstCaseTreeCacheKnownPaths {
// Then we have to get the tree
if !self.cached_merks.contains(path) {
cost_return_on_error_no_add!(
&cost,
cost,
GroveDb::add_worst_case_get_merk_at_path::<RocksDbStorage>(
&mut cost,
path,
Expand Down Expand Up @@ -247,7 +247,7 @@ impl<G, SR> TreeCache<G, SR> for WorstCaseTreeCacheKnownPaths {
// Then we have to get the tree
if !self.cached_merks.contains(&base_path) {
cost_return_on_error_no_add!(
&cost,
cost,
GroveDb::add_worst_case_get_merk_at_path::<RocksDbStorage>(
&mut cost,
&base_path,
Expand Down
14 changes: 6 additions & 8 deletions grovedb/src/batch/just_in_time_reference_update.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,17 @@
F: FnMut(&[Vec<u8>], bool) -> CostResult<Merk<S>, Error>,
S: StorageContext<'db>,
{
pub(crate) fn process_old_element_flags<G, SR>(
key: &[u8],
serialized: &[u8],
new_element: &mut Element,
old_element: Element,
old_serialized_element: &[u8],
is_in_sum_tree: bool,
flags_update: &mut G,
split_removal_bytes: &mut SR,
grove_version: &GroveVersion,
) -> CostResult<CryptoHash, Error>

Check warning on line 38 in grovedb/src/batch/just_in_time_reference_update.rs

View workflow job for this annotation

GitHub Actions / clippy

this function has too many arguments (9/7) 

warning: this function has too many arguments (9/7)
  --> grovedb/src/batch/just_in_time_reference_update.rs:28:5
   |
28 | /     pub(crate) fn process_old_element_flags<G, SR>(
29 | |         key: &[u8],
30 | |         serialized: &[u8],
31 | |         new_element: &mut Element,
...  |
37 | |         grove_version: &GroveVersion,
38 | |     ) -> CostResult<CryptoHash, Error>
   | |______________________________________^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
   = note: `#[warn(clippy::too_many_arguments)]` on by default
where
G: FnMut(&StorageCost, Option<ElementFlags>, &mut ElementFlags) -> Result<bool, Error>,
SR: FnMut(
Expand All @@ -53,13 +53,13 @@
updated_new_element_with_old_flags.set_flags(maybe_old_flags.clone());
// There are no storage flags, we can just hash new element
let new_serialized_bytes = cost_return_on_error_no_add!(
&cost,
cost,
updated_new_element_with_old_flags.serialize(grove_version)
);
let val_hash = value_hash(&new_serialized_bytes).unwrap_add_cost(&mut cost);
Ok(val_hash).wrap_with_cost(cost)
} else {
let val_hash = value_hash(&serialized).unwrap_add_cost(&mut cost);

Check warning on line 62 in grovedb/src/batch/just_in_time_reference_update.rs

View workflow job for this annotation

GitHub Actions / clippy

this expression creates a reference which is immediately dereferenced by the compiler 

warning: this expression creates a reference which is immediately dereferenced by the compiler
  --> grovedb/src/batch/just_in_time_reference_update.rs:62:47
   |
62 |                     let val_hash = value_hash(&serialized).unwrap_add_cost(&mut cost);
   |                                               ^^^^^^^^^^^ help: change this to: `serialized`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
   = note: `#[warn(clippy::needless_borrow)]` on by default
Ok(val_hash).wrap_with_cost(cost)
}
} else {
Expand Down Expand Up @@ -93,7 +93,7 @@
updated_new_element_with_old_flags.set_flags(maybe_old_flags.clone());

let serialized_with_old_flags = cost_return_on_error_no_add!(
&cost,
cost,
updated_new_element_with_old_flags.serialize(grove_version)
);
KV::node_value_byte_cost_size(
Expand All @@ -115,7 +115,7 @@
if let Some(old_element_flags) = maybe_old_flags.as_mut() {
if let BasicStorageRemoval(removed_bytes) = storage_costs.removed_bytes {
let (_, value_removed_bytes) = cost_return_on_error_no_add!(
&cost,
cost,
split_removal_bytes(old_element_flags, 0, removed_bytes)
);
storage_costs.removed_bytes = value_removed_bytes;
Expand All @@ -125,7 +125,7 @@
let mut new_element_cloned = original_new_element.clone();

let changed = cost_return_on_error_no_add!(
&cost,
cost,
(flags_update)(
&storage_costs,
maybe_old_flags.clone(),
Expand All @@ -145,10 +145,8 @@
return Ok(val_hash).wrap_with_cost(cost);
} else {
// There are no storage flags, we can just hash new element
let new_serialized_bytes = cost_return_on_error_no_add!(
&cost,
new_element_cloned.serialize(grove_version)
);
let new_serialized_bytes =
cost_return_on_error_no_add!(cost, new_element_cloned.serialize(grove_version));

new_storage_cost = KV::node_value_byte_cost_size(
key.len() as u32,
Expand Down
Loading
Loading