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 remove action apply with early iteration exit #424

Closed
houqp opened this issue Sep 7, 2021 · 2 comments · Fixed by #431
Closed

Optimize remove action apply with early iteration exit #424

houqp opened this issue Sep 7, 2021 · 2 comments · Fixed by #431
Assignees
Labels
binding/rust Issues for the Rust crate enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@houqp
Copy link
Member

houqp commented Sep 7, 2021

Description

Use Case

Right now, we use Iterator::retain to remove files from active file list when applying remove actions from the transaction log:

state.files.retain(|a| *a.path != v.path);

This can be further optimized by performing an early exit of the iteration when we reach the file that needs to be removed. There is no point going through the remaining of the file list and compare paths since file paths should be unique.

Related Issue(s)

@houqp houqp added enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers binding/rust Issues for the Rust crate labels Sep 7, 2021
@akshay26031996
Copy link
Contributor

Hey, can I have this issue? I am thinking of the following approach.

First find the position of the file to remove.

state.files.iter().position(|a| *a.path == v.path)

It will stop processing as soon as it finds a true.

Then we can just use the index from above to remove the element from files.

let index = { state.files.iter().position(|a| *a.path == v.path) };
if let Some(index) = index {
    state.files.remove(index);
}

@houqp
Copy link
Member Author

houqp commented Sep 16, 2021

Looks good to me, @akshay26031996 assigned the issue to you.

houqp pushed a commit that referenced this issue Sep 18, 2021
* Optimize remove action apply using position and remove

* Changing remove to swap_remove
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
binding/rust Issues for the Rust crate enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants