8000 Replace `for_each_mut` (internal iteration) with `iter_mut` (external iteration) in the AMT · Issue #1996 · filecoin-project/ref-fvm · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Replace for_each_mut (internal iteration) with iter_mut (external iteration) in the AMT #1996

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
Open
Stebalien opened this issue Feb 20, 2024 · 13 comments
Open

Replace for_each_mut (internal iteration) with iter_mut (external iteration) in the AMT #1996

Stebalien opened this issue Feb 20, 2024 · 13 comments
Assignees
@Sahilgill24
Labels
good first issue Good for newcomers Kind: Improvement An improvement of something that exists.

Comments

@Stebalien
Copy link
Member
Stebalien commented Feb 20, 2024 •
edited
Loading

The iterator will have to yield a smart pointer that:

  1. Can be dereferenced immutably with no effect.
  2. Can be dereferenced mutably, marking the value as changed.
  3. Can be deleted (e.g., by calling some form of ptr.delete() method). This method should consume the smart pointer (possibly returning the inner value).

This will get rid of the last "internal" iteration feature.
@Stebalien Stebalien added Kind: Improvement An improvement of something that exists. good first issue Good for newcomers labels Feb 20, 2024
@Stebalien Stebalien mentioned this issue Feb 20, 2024
Closed
@rjan90 rjan90 added this to FilOz Feb 22, 2024
@rjan90 rjan90 moved this to 🐱Todo in FilOz Feb 22, 2024
@rjan90 rjan90 moved this from 🐱Todo to 📌 Triage in FilOz Apr 5, 2024
@PhantomOz
Copy link

can i work on this?

@Stebalien
Copy link
Member Author

Absolutely! Take a look at how we currently use it here.

@rjan90 rjan90 moved this from 📌 Triage to ⌨️In Progress in FilOz Apr 13, 2024
@rjan90
Copy link
Contributor
rjan90 commented Aug 21, 2024

Hey @PhantomOz! Are you still interested in working on this, or should I unassign you?

@rjan90
Copy link
Contributor
rjan90 commented Sep 4, 2024

Unassigning since I have not hear back yet.

@rjan90 rjan90 moved this from ⌨️In Progress to 🐱Todo in FilOz Sep 4, 2024
@PhantomOz
Copy link

I apologize didn't notice that i was assigned till now, Please reassign so that I can work on it @rjan90

@rjan90
Copy link
Contributor
rjan90 commented Sep 5, 2024

No worries! Reassigned to you now 👍

@rjan90 rjan90 moved this from 🐱Todo to ⌨️In Progress in FilOz Sep 5, 2024
@rjan90
Copy link
Contributor
rjan90 commented Sep 24, 2024

Hey @PhantomOz! Do you need some additional guidance on this issue ticket? How is the progress on going on this?

@PhantomOz
Copy link

@rjan90 Yes I do

@rjan90 rjan90 moved this from ⌨️ In Progress to 🐱 Todo in FilOz Oct 4, 2024
@BigLep BigLep moved this from 🐱 Todo to 👊 Needs Commitment in FilOz Mar 17, 2025
@Sahilgill24
Copy link

Hey @rjan90 , If the issue is still open to be worked upon , Can i give it a try ?

@rjan90
Copy link
Contributor
rjan90 commented May 2, 2025

Hey @Sahilgill24! Yes, it is still open to be worked on. Go ahead and give it a try, and feel free to ask any questions you may have

@Sahilgill24
Copy link
Sahilgill24 commented May 2, 2025 •
edited
Loading

Hey @rjan90
My understanding of the issue is that currently for_each_mut() is based on internal iteration and works like this amt.for_each_mut(f) where the function for_each_mut calls the for_each_while_mut() inside of it , If The function iter_mut() is based on external iteration so it may look like this

for i in amt.iter_mut() { 
       *(i.derefmut())
 }

where i is the custom smart pointer and f is the closure.
Could you please point out if there is any mistake in my understanding of the issue

@Stebalien
Copy link
Member Author

Yes, assuming the smart pointer i implements DerefMut (and Deref).

But note: this is a good first issue because it's well specified, not because it's easy. You're going to have to play some games with Rc and RefCell because the smart pointer cannot borrow from the iterator.

@Sahilgill24
Copy link

Hey @Stebalien , so I was able to Implement the iter_mut function and successfully write the test for it but I have used Box::leak() which holds a value for the entire runtime but also causes leak in the memory , I will try to implement the same using Rc and Refcell also once .

    // Amtptr is the custom smart pointer 
    pub fn iter_mut<'a>(&mut self) -> impl Iterator<Item = (u64, Amtptr<'a, V>)>
    where
        V: 'a + Clone,
    {
        let mut handles = Vec::new();
        let mut iterable = Vec::new();

        self.for_each_while_mut(|idx, val| {
            let val2 = val.clone();
            handles.push((idx, val2));

            Ok(true)
        });

        for i in handles {
            let tempval = Box::leak(Box::new(i.1));
            iterable.push((i.0, Amtptr::new(tempval)));
        }
        iterable.into_iter()
    }

This function can be used like this

    for ptr in new_amt.iter_mut() {
        let current_idx = ptr.0;
        let mut val = ptr.1;
        f(current_idx, val.deref_mut()).unwrap();
        // f is a closure , which we can define earlier

    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Sahilgill24 Sahilgill24

Labels
good first issue Good for newcomers Kind: Improvement An improvement of something that exists.
Projects
FilOz
Status: 👊 Needs Commitment
Milestone
No milestone
Development

No branches or pull requests
4 participants
@Stebalien @rjan90 @PhantomOz @Sahilgill24
0