Skip to content

Conversation

CyrusNajmabadi
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi commented May 17, 2024

Drops CPu utilization in Devenv from 57Msecs in an expensive FAR scenario:

image

to just 34Msecs

image

--

While the FAR window is good, it's not good enough that thousands of notification updates dont' impact it.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner May 17, 2024 07:02
@ghost ghost added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels May 17, 2024
@CyrusNajmabadi
Copy link
Member Author

@ToddGrun ptal

// Similarly, updating the actual FAR window can be quite expensive (especially when there are thousands of
// results). To limit the amount of work we do, we'll only update the window every 500ms.
_notifyQueue = new AsyncBatchingWorkQueue(
DelayTimeSpan.Medium,
Copy link
Contributor

Choose a reason for hiding this comment

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

DelayTimeSpan.Medium

I kind of feel like I wish the ABWQ had an option where it would customize the delay as the work expanded.

Something like:
For the first 1 second, it updates on NearImmediate cadence.
Between 1 - 5 seconds, it updates on short cadence
After 5 seconds, it updates on medium cadence

It just feels like adding a 500 ms delay on really quick FAR requests might be perceptible.

Copy link
Member Author

Choose a reason for hiding this comment

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

I def didn't notice anything feeling off here. I could make it 250ms if you feel better about that. We'll still likely get the majority of the benefit.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would like that. 250 ms is a lot less noticeable than 500 ms.

@CyrusNajmabadi
Copy link
Member Author

@ToddGrun done

@CyrusNajmabadi CyrusNajmabadi enabled auto-merge May 17, 2024 16:33
Copy link
Contributor

@ToddGrun ToddGrun left a comment

Choose a reason for hiding this comment

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

:shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants