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

Conversation

Adlai-Holler
Copy link
Contributor

@appleguy @nguyenhuy Been majorly frustrated at work lately so it feels super good to have a nice, clean improvement to offer.

@Adlai-Holler
Copy link
Contributor Author

Also for sugar on top, I was hoping to use QOS_CLASS_SOMETHING to specify the global queue there. The docs for dispatch_get_global_queue say You may specify the values QOS_CLASS_USER_INTERACTIVE, QOS_CLASS_USER_INITIATED, QOS_CLASS_UTILITY, or QOS_CLASS_BACKGROUND.. This feels like QOS_CLASS_UTILITY to me but I want to know what y'all think

@levi
Copy link
Contributor

levi commented Oct 5, 2015

Awesome stuff, @Adlai-Holler!

@Adlai-Holler
Copy link
Contributor Author

Thanks man!

Copy link
Contributor

Choose a reason for hiding this comment

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

Create a local variable, CGSize calculatedSize = [node measureWithSizeRange:constrainedSize];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah! Good point

@appleguy
Copy link
Contributor

appleguy commented Oct 5, 2015

Very nice, thank you @Adlai-Holler! Some of @levi's changes are a possible performance regression in this code, but hopefully we can clean that up in the time before it lands — and having this as a higher bar will inspire us to do so :).

I would suggest we not touch queue priorities for now. The dispatch headers actually talk about the risk of priority inversion and in at least one place recommend not using priorities unless absolutely necessary. The main thread already runs at a higher priority, so there is no real risk of dropping frames from this background thread work. More importantly, if we run at lower priority, it's likely that arbitrary developers' own background tasks (compressing analytics data to send over the network, JSON parsing, etc) could be taking CPU time away from layout, which the user may well be waiting on (at the end of the scrollable area, awaiting for new cells to finish layout and be added).

appleguy added a commit that referenced this pull request Oct 5, 2015
Faster handling of iteration to lay out nodes in ASDataController.
@appleguy appleguy merged commit 2f1d57e into facebookarchive:master Oct 5, 2015
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.

4 participants