Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump mac editor and implement CocoaTextBufferVisibilityTracker #65643

Conversation

sandyarmstrong
Copy link
Member

@sandyarmstrong sandyarmstrong commented Nov 28, 2022

This should help reduce the number classification marks triggering in the editor in VSMac.

Fixes https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1689011

Backport of #65568 without the refactoring.

This should help reduce the number classification marks triggering in
the editor in VSMac.

Fixes https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1654358

Backport of #65568 without
the refactoring.
@sandyarmstrong
Copy link
Member Author

@CyrusNajmabadi
Copy link
Member

Were you guys able to try this out? in particular, it would be really valuable to ensure that the visbiility checks are actually working in vs4mac at runtime. and that you see that the non-visible tabs now get throttled to one update every 3 seconds per tagger.

@sandyarmstrong
Copy link
Member Author

This version of the change (on top of main, in the older PR) is what Marius was testing locally with good results. I'll be testing a build from this branch today.

namespace Microsoft.CodeAnalysis.Workspaces
{
[Export(typeof(ITextBufferVisibilityTracker))]
internal sealed class CocoaTextBufferVisibilityTracker : ITextBufferVisibilityTracker
Copy link
Member

Choose a reason for hiding this comment

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

i haven't looked closely. but the presumption is that this is a straight port, just tweaked fort he cocoa visibility checks. if so, this is ok with me. if anything else was changed, lmk. thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, that is the intent! Thank you!

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, it's just the commit I initially made I think. And that was the changeset, I think the diff itself is just removing .VisualElement and changing the event handler type.

@CyrusNajmabadi
Copy link
Member

LGTM then given the manual validation as well :)

@Therzok
Copy link
Contributor

Therzok commented Nov 28, 2022

and that you see that the non-visible tabs now get throttled to one update every 3 seconds per tagger.

My speedscope trace with the insertion PR doesn't contain the problematic frames, so I assume that's fine!
pointofsale-inserted-semantic-highlighting.speedscope.json.zip

@davidwengier
Copy link
Contributor

@arkalyanms since this is targetting 17.4

@arunchndr
Copy link
Member

Thanks, I'll follow up with @sandyarmstrong on https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1689011 to make the 17.4 servicing happen.

@sandyarmstrong sandyarmstrong changed the base branch from release/dev17.4 to release/vsmac17.4 November 29, 2022 01:02
@davidwengier davidwengier merged commit adf0434 into release/vsmac17.4 Nov 29, 2022
@davidwengier davidwengier deleted the dev/therzok/cocoa-text-buffer-visibility-tracker-17.4 branch November 29, 2022 03:43
@davidwengier davidwengier restored the dev/therzok/cocoa-text-buffer-visibility-tracker-17.4 branch November 29, 2022 03:43
@JoeRobich JoeRobich deleted the dev/therzok/cocoa-text-buffer-visibility-tracker-17.4 branch May 7, 2024 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants