vscode workspaces don't play nicely with r-a

[Veykril]

When using a code workspace where a cargo workspace is split up into multiple folder views, r-a will duplicate the loaded cargo workspace for each folder currently which leads to excessive memory usage. We should investigate if we can prevent this from happening somehow

[Veykril]

So basically I think the server should try to dedup/map workspace folders that indirectly point to the same top-level cargo workspace. We discover them here https://github.com/veykril/rust-analyzer/blob/3c7b6716d1ab458664a8b372b79347b9a9cacb98/crates/rust-analyzer/src/config.rs#L753 where workspace roots are the folders supplied by VSCode.

[zaurask]

Short answer (realized while writing long answer): Looking into your fix #14603 I recognized that

rust-analyzer/crates/rust-analyzer/src/reload.rs

Lines 215 to 227 in 112464f

	let mut i = 0;
	while i < workspaces.len() {
	if let Ok(w) = &workspaces[i] {
	if let Some(dupe) = workspaces[i + 1..]
	.iter()
	.filter_map(|it| it.as_ref().ok())
	.position(|ws| ws.eq_ignore_build_data(w))
	{
	_ = workspaces.remove(dupe);
	}
	}
	i += 1;
	}

is not working as intended through two problems:

1. Position returns the index inside the slice which you are then using to remove from the whole vec which results in removing the wrong element.
2. To remove duplicates every element has to be compared to all others. Currently through advancing i and slicing, all elements with index < i are not taken into account for subsequent comparisons.

Possible fix:

while i < workspaces.len() {
    if let Ok(w) = &workspaces[i] {
        match workspaces
            .iter()
            .filter_map(|it| it.as_ref().ok())
            .enumerate().filter(|&(idx, _)| idx != i).map(|(_, v)| v)
            .position(|ws| ws.eq_ignore_build_data(w))
        {
            Some(dupe) => _ = workspaces.remove(dupe),
            None => i += 1,
        }
    } else {
        i += 1;
    }
}

Long answer:

Coincidentally I have also been looking at this issue (for learning purposes) and I suspect your fix #14603 does not correct the excess memory usage through duplicated cargo workspaces.

My test setup is a vscode workspace with folders for each crate from rust-analyzer basically duplicating the cargo workspace. With your fix memory usage still grows to 6.9 GB in the vscode workspace compared to ~870 MB when opening the cargo workspace under vscode.

Debugging the vscode workspace indicates that

rust-analyzer/crates/rust-analyzer/src/reload.rs

Lines 215 to 227 in 112464f

	let mut i = 0;
	while i < workspaces.len() {
	if let Ok(w) = &workspaces[i] {
	if let Some(dupe) = workspaces[i + 1..]
	.iter()
	.filter_map(|it| it.as_ref().ok())
	.position(|ws| ws.eq_ignore_build_data(w))
	{
	_ = workspaces.remove(dupe);
	}
	}
	i += 1;
	}

does not remove duplicate cargo workspaces properly since the crates from rust-analyzer clearly belong to only one cargo workspace.

[Veykril]

1.) You are right, if we dedup we are moving two indices instead of one (classic modifying collection while iterating it mistake)
2.) This should be fine, we are skipping comparisons of elements that we already compared for the given index, that is we compare the first element with everything else, the second with everything but the first (which is fine, since we compared it with the first when comparing for the first element), etc

[Veykril]

Debugging the vscode workspace indicates that ... does not remove duplicate cargo workspaces properly since the crates from rust-analyzer clearly belong to only one cargo workspace.

Can you explain this more? It did remove duplicates in my testing, I used a vscode workspace that had 2 folders opened pointing to 2 crates of a cargo workspace resulting in rust-analyzer deduplicating the loaded projects to the cargo workspace once instead of twice.

[zaurask]

1.) You are right, if we dedup we are moving two indices instead of one (classic modifying collection while iterating it mistake) 2.) This should be fine, we are skipping comparisons of elements that we already compared for the given index, that is we compare the first element with everything else, the second with everything but the first (which is fine, since we compared it with the first when comparing for the first element), etc

In 2) position() short-circuits, so if one match is found all other elements are not compared to and the index is advanced. Details about my debugging I can explain, when I am at my computer.

[Veykril]

In 2) position() short-circuits, so if one match is found all other elements are not compared to and the index is advanced

Ah, no need to give me details now I understand what you were trying to get at. Yes you are correct this is also a bug.