Explorer: move off iterating over file changes

[bpasero]

The FileChangesEvent is now internally using a ternary search tree which makes the contains method very efficient. As such I went ahead and deprecated those methods of the event that would expose the raw changes that are not optimal to be dealt with.

This issue is to see what is needed from the file change event API to move the explorer off these deprecated methods so that we ensure no expensive work from reacting to file changes when many changes occur.

The users are:

vscode/src/vs/workbench/contrib/files/common/explorerService.ts

Line 299 in b1c54c1

const added = e.getAdded();

vscode/src/vs/workbench/contrib/files/common/explorerService.ts

Line 329 in b1c54c1

const deleted = e.getDeleted();

vscode/src/vs/workbench/contrib/files/common/explorerService.ts

Line 344 in b1c54c1

const updated = e.getUpdated();

vscode/src/vs/workbench/contrib/files/common/explorerService.ts

Line 367 in b1c54c1

return e.filter(change => {

[isidorn]

Explorer uses these methods in the following way getAdded, getDeleted, e.filter are about trying to figure out if a file change requires the Explorer tree to refresh.
getUpdated similar to the above, but only for the case that we are using a modified sort order, so a re-sorting might be needed, thus we refresh.

If we want to move off these methods and only use contains then the alternative strategy would be to go over all the resolved explorer items and for each check if the event contains them. If yes then a refresh of the tree is needed. Please note that there can be a lot of resolved explorer items, however if the contains is blazing fast I guess that would not be a problem.

Anyways let me know what you think about my proposal.

[bpasero]

@isidorn how does it work today, are you not already going over all the resolved explorer items or do you have a fast map for the explorer items too?

I guess it boils down to this: if there are tons of resolved explorer items, it would be faster to iterate over the file changes and otherwise the other way around, correct?

[isidorn]

@bpasero currently it works: I just get all added resources from the file event and then I have a map on my side which looks up pretty fast.

Correct, if there are a lot of resolved explorer items it is better if the event is rich and has the deprecated methods, so I can just go through all teh change and pass it to my explorer item maps.
If there are not a lot of resolved explorer items than for each item I could check if it is contained in the event.

[bpasero]

Maybe a strategy would be to check changes.length and compare that with the explorer items to decide wether to use the one or the other way depending on wether the file events are a lot more than explorer items.

[isidorn]

Ok, let me assign to November to experiment with idea then.