Scroll performance while creating a bunch of files

[TylerLeonhardt]

Testing #133753

So I created a folder in vscode called foo and make sure I "open it" with the twisty so that files under it would be shown. then ran this script which simply:

• creates 10000 files
• waits 30s
• and then removes them all:

1..10000 | % {
    $_ > "$_.txt"
}
sleep 30
1..10000 | % {
    Remove-Item "$_.txt"
}

Scroll performance is pretty rough when the creation is happening. Once the creation stops happening, then perf returns back to normal almost instantly.

[bpasero]

@TylerLeonhardt scroll performance in the explorer or where? Can you run a perf profile and attach it? Maybe run out of sources too to get good method names.

Also what platform was this tested on?

[TylerLeonhardt]

scroll performance in the explorer or where?

Yeah in the explorer. This was on macOS.

Can you run a perf profile and attach it? Maybe run out of sources too to get good method names.

Can do.

[TylerLeonhardt]

This is the first time I'm using the profile feature so just lmk if there's anything I can do to be more helpful.
Profile-20210929T110623.json.zip

[bpasero]

Thanks, I tried to reproduce this with a simple node.js script:

const fs = require("fs");

for (let i = 0; i < 10000; i++) {
	fs.writeFileSync(`<PATH TO VSCODE>/foo/${i}.txt`, 'Hello World');
}

setTimeout(() => {
	for (let i = 0; i < 10000; i++) {
		fs.unlinkSync(`<PATH TO VSCODE>/foo/${i}.txt`);
	}
}, 30000);

And scrolling was super snappy even when foo was expanded and indicating all the files coming in.

The profile I created didn't show anything suspicious.

Can you maybe do a profile again, this time via the classic tools:

You might have to bring that one in via:

Also: is this a fresh instance, with no special settings or extensions?

[TylerLeonhardt]

it was.... pretty smooth that time but had moments where the scroll wasn't fluid. I don't have any extensions or settings installed. I'm running out of sources. I feel like maybe the other programs on my machine could be hurting it.

CPU-20210930T120002.cpuprofile.zip

[bpasero]

idk, I see most time is spend in the scrolling callback getting decorations:

[jrieken]

I'll use that as a chance to re-measure if all the allocation avoiding the tst is doing is actually worth it or if we should start splitting strings.

[jrieken]

It's actually two things: one is the performance of the search tree and second is how the decoration service organizes its data. Internally each provider has its own search tree which was OK when it started because there were only two providers, not it is at least 10. So, whatever the search tree costs needs to be multiplied by 10.

[jrieken]

So, while rendering is hot I believe it's not the cause any of the stalls/freezes. There is a lot going on with rendering but we only render a few items on screen and therefore only ask for decorations of a few items (32 on my screen/sizing). It all happens in nice chunks of work. I do see one long (~1sec) plateau in the profile and that's when 10 thousand file events are being proceed. It also happens to be the search tree and @bpasero I wonder if you have adopted the fill-function? (That's the workaround for the tree not rebalancing)

[jrieken]

I wonder if you have adopted the fill-function? (That's the workaround for the tree not rebalancing)

Nevermind - its consumer checking if they are affected by some changes. Most time is actually spend in doing the "ignore case compare". Maybe it is faster to create throw-away strings and compare them for equality than doing what we do.

es

[jrieken]

Actually still interested in how the search tree for events is created. On the measuring front it turns out that our custom compare ignore case is faster but only marginally and not in safari: https://jsben.ch/FvFx7

[jrieken]

So, tweaking how the event is build does matter. I adopted the fill-function for file events (after changing it so that it actually fits here) and handling deletes is now at 100ms (from ~1sec). That's because we likely avoid the tree becoming a flat list here (it's a workaround for not rebalancing but given the tree is build and thrown out it might actually be a good solution...). Changes are in fillFileEvents, this commit 41be57c

[bpasero]

@jrieken cool findings, yeah I forgot to adopt the fill function when you added it.

2 milestones ago I was making a lot of changes to introduce throttling in the renderer and cut off events if a very large number occurs:

vscode/src/vs/platform/files/common/fileService.ts

Lines 977 to 982 in dbeff88

	private static readonly FILE_EVENTS_THROTTLING = {
	maxChangesChunkSize: 500 as const, // number of changes we process per interval
	maxChangesBufferSize: 30000 as const, // total number of changes we are willing to buffer in memory
	coolDownDelay: 200 as const, // rest for 100ms before processing next events
	warningscounter: 0 // keep track how many warnings we showed to reduce log spam
	};

Still, the tst is created in the renderer as before:

vscode/src/vs/platform/files/common/files.ts

Lines 665 to 688 in 33c149f

	constructor(changes: readonly IFileChange[], ignorePathCasing: boolean) {
	for (const change of changes) {
	switch (change.type) {
	case FileChangeType.ADDED:
	if (!this.added) {
	this.added = TernarySearchTree.forUris<IFileChange>(() => ignorePathCasing);
	}
	this.added.set(change.resource, change);
	break;
	case FileChangeType.UPDATED:
	if (!this.updated) {
	this.updated = TernarySearchTree.forUris<IFileChange>(() => ignorePathCasing);
	}
	this.updated.set(change.resource, change);
	break;
	case FileChangeType.DELETED:
	if (!this.deleted) {
	this.deleted = TernarySearchTree.forUris<IFileChange>(() => ignorePathCasing);
	}
	this.deleted.set(change.resource, change);
	break;
	}
	}
	}

One thing we have not yet explored: what if the tst is actually created in the file watcher process and then serialised over? That would require some kind of toJSON / fromJSON method on the tst but would actually offload all the compute for creating the tree to another process. It would probably not help much for the reading side though, but that is hopefully not the bottleneck anyway.

[jrieken]

what if the tst is actually created in the file watcher process and then serialised over?

I don't think that will help. Building the tst isn't the bottleneck, e.g it takes ~15ms for 10K deletes (see below). The problem occurs when it is build to a skinny shape so that it basically morphs into a list

[jrieken]

Reworked how the decoration service organizes data, with that we will only check the TST once per uri and per provider. Also, pushed 21991a0 which ensures that the TST is balanced with respect to the left/right nodes. It can still be skinny along the mid-path but I don't see how that can be avoided...

We can still geek out further and try to understand why isItemVisible is so slow (I see a ton of calls to path#relative and normalize which I believe shouldn't be needed) and it's also interesting that FileEvent#doContains has such a high self-time. Anyways, closing this issue and @TylerLeonhardt or @bpasero decide if you want to optimize things further

[connor4312]

Things seem to perform well for me. I did this in a git repo so that the files would get git decorations and the renderer was smooth the entire time