Have throttled worker always wait throttleDelay between work calls

[lramos15]

Consider the following case, maxWorkChunkSize of 1 and a throttleDelay of 1000ms. The maxBufferedWork doesn't matter here.

1. Queue a unit at 0ms. It immediately executes as there is nothing in the throttler
2. Queue a unit at 500ms. It immediately executes again as there is nothing in the throttler.

This seems wrong as I don't want my work items to execute without a minimum 1000ms gap in between them. Instead even if the queue is empty, the throttled worker should ensure that the second queue task must wait 500ms before executing so it's been 1000ms since the last unit of work.

This does modify the text behavior slightly, so I am happy to make this an option on the worker options or something, but I was surprised when the worker did not work out of the box like this

[bpasero]

I think this change would not be suitable for the case I created the ThrottledWorker for: if now a file watcher sends 1 event every 10ms, we would artificially wait 200ms before letting this event go. The intent was to start throttling when an unusual large number of events comes in at the same time:

vscode/src/vs/platform/files/node/watcher/parcel/parcelWatcher.ts

Lines 176 to 183 in b591f33

	private readonly throttledFileChangesEmitter = this._register(new ThrottledWorker<IFileChange>(
	{
	maxWorkChunkSize: 500, // only process up to 500 changes at once before...
	throttleDelay: 200, // ...resting for 200ms until we process events again...
	maxBufferedWork: 30000 // ...but never buffering more than 30000 events in memory
	},
	events => this._onDidChangeFile.fire(events)
	));

[lramos15]

Would it be worth a throttled worker option or do you think I should just use a different utility?

Here's the PR where I try to consume this if interested. https://github.com/microsoft/vscode-copilot/pull/8758

[lramos15]

Update, I've made a change to put this work in IThrottledWorkerOptions

[bpasero]

@lramos15 yeah we can have it with that option, pushed some cleanup.

Didn't you have a test for this in your first PR?

[lramos15]

I did not create a test for it in my first PR because it modified the default behavior so instead I modified your existing tests. I will create a test for this and get another ✔️ since you did the cleanup. Thanks for the 💄 and review