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.

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

Optimize moveos_stdlib::table #908

Merged
merged 5 commits into from
Oct 4, 2023
Merged

Conversation

pause125
Copy link
Collaborator

@pause125 pause125 commented Oct 4, 2023

Summary

  1. Destroy the table if the value of table has the drop ability. Close [MoveosStd] Support drop Table<K,V> if the V has drop ability #899
  2. Add interface of table::length and table::is_empty, support destroy empty table. Close [MoveosStd] Table need provide destroy_empty_table, is_empty and length function  #342
  3. Fix bug when table::destroy non-empty tables. Close [Bug] moveos_stdlib::table::destroy can be called with non-empty tables. #320

@vercel
Copy link

vercel bot commented Oct 4, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
rooch ⬜️ Ignored (Inspect) Visit Preview Oct 4, 2023 6:20am

@@ -34,6 +34,9 @@ pub trait StateResolver {
cursor: Option<Vec<u8>>,
limit: usize,
) -> Result<Vec<Option<ListState>>, anyhow::Error>;

/// Resolve the table size if the handle is a table.
fn resolve_size(&self, handle: &ObjectID) -> Result<u64, anyhow::Error>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not need a resolve_size in the StateResolver trait. We can use resolve_state(GLOBAL_OBJECT_STORAGE_HANDLE, table_handle) -> State(TableInfo) to replace it.

We also can refactor this later. Maybe the StateResolver need to be refactor to:

pub trait StateResolver {
  // get object data from global state tree.
  fn resolve_object(&self, object: &ObjectID) -> Result<Option<Object>, anyhow::Error>;
  fn resolve_table(&self, handle: &ObjectID, key: &[u8]) -> Result<Option<State>, anyhow::Error>;

Copy link
Collaborator Author

@pause125 pause125 Oct 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recorded an issue. Refactoring StateResolver may relate too many places, I will check that later.

Comment on lines 133 to 135
native fun destroy_empty_box_unchecked(table_handle: &ObjectID);

native fun drop_unchecked_box(table_handle: &ObjectID);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge the drop_unchecked_box and destroy_empty_box_unchecked?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@jolestar jolestar merged commit 5e338a7 into rooch-network:main Oct 4, 2023
4 checks passed
@pause125 pause125 deleted the table_length branch November 3, 2023 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants