Skip to content
This repository has been archived by the owner on Feb 2, 2023. It is now read-only.

[ASRangeController] Minimize number of registrations to node display notifications #1864

Merged
merged 1 commit into from
Jul 8, 2016

Conversation

nguyenhuy
Copy link
Contributor

@nguyenhuy nguyenhuy commented Jul 8, 2016

Previously, ASRangeController registers to notification center every time a range update is performed, regardless of the number of nodes that need to schedule for a recursive display in response to the update.

Instead, if this number is taken into account and range controller doesn't observe further notifications if it is zero, we can minimize the number of range updates executed and may fix #1460 (verification needed though).

@appleguy
Copy link
Contributor

appleguy commented Jul 8, 2016

@nguyenhuy thanks!!

@maicki
Copy link
Contributor

maicki commented Jul 8, 2016

@nguyenhuy LGTM

Other thinking around that is, any opinion about looking into finding a way to not align on NSNotifications anymore at all and remove the NSNotificationCenter overhead?

@Adlai-Holler
Copy link
Contributor

Without interfering with Michael's question I'm going to merge this so that it's in the bag.

@Adlai-Holler Adlai-Holler merged commit 9154fbb into facebookarchive:master Jul 8, 2016
@nguyenhuy nguyenhuy deleted the range_controller branch July 9, 2016 05:00
@nguyenhuy
Copy link
Contributor Author

@Adlai-Holler Thanks!

@maicki: Will keep thinking about it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ASRenderingEngineDidDisplayScheduledNodes notification posted, but another ASTableView catches it
4 participants