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

1. Change the inspector to a virtual list. #715

Merged
merged 2 commits into from
Dec 29, 2024

Conversation

jm-observer
Copy link
Contributor

No description provided.

Copy link
Collaborator

@jrmoulton jrmoulton left a comment

Choose a reason for hiding this comment

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

What is the motivation for making this change?

Comment on lines +131 to +154
fn total(&self) -> usize {
match &self.ty {
DataType::Internal { expanded, children } => {
if expanded.get() {
let mut total = 1;
for child in children {
total += child.total();
}
total
} else {
1
}
}
DataType::Leaf => 1,
}
}

fn get_children(
&self,
next: &mut usize,
min: usize,
max: usize,
level: usize,
) -> Vec<(usize, usize, CapturedData)> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like both of these functions are relatively expensive to compute. Is there a reason that these aren't just computed and stored in the init function? Then the get children function could just return a slice without needing to build a new vec of children elements.

@jm-observer
Copy link
Contributor Author

What is the motivation for making this change?

After upgrading Floem to version 0.2.0, using the inspector causes the program to overflow.

It looks like both of these functions are relatively expensive to compute. Is there a reason that these aren't just computed and stored in the init function? Then the get children function could just return a slice without needing to build a new vec of children elements.

As far as I know, a virtual list is relatively inexpensive and dynamically calculated.

Additionally, the example of the virtual list is overly simplistic.

@jrmoulton
Copy link
Collaborator

I should've tested it first. I thought this would be slower than the old way with rebuilding a list of items every time the list refreshes but it's definitely faster.

@jrmoulton jrmoulton merged commit 3729bac into lapce:main Dec 29, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants