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

amp-bind: Binding limits vs. infinite scroll #15519

Closed
dreamofabear opened this issue May 23, 2018 · 6 comments
Closed

amp-bind: Binding limits vs. infinite scroll #15519

dreamofabear opened this issue May 23, 2018 · 6 comments

Comments

@dreamofabear
Copy link

Context: #11434 (comment)

amp-bind's bindings limit will be problematic once infinite scroll or appending "view more" use cases become more prevalent.

One of the original reasons why these limits exist is (background) initialization latency for amp-bind. If parsing an expression takes O(1ms) on modest hardware, then 2000 bindings means the AMP page could have up to 2s time-to-interactive (TTI) WRT amp-bind.

For infinite scroll/view more, the incremental markup added per trigger is typically small. So we likely need not worry about TTI. Instead, we should think about the total evaluation time for updating the page with a larger number of bindings when AMP.setState() is invoked. Luckily, evaluating expressions are typically much faster than parsing them [1], so perhaps something like the following could work:

  • Rework 1000 binding limit to only apply to initial DOM scan
  • Allow X >> 1000 total binding limit -- includes runtime changes e.g. infinite scroll/view more
  • Allow Y << 1000 incremental binding limit -- which constrains the number of bindings that may be added per infinite scroll/view more trigger [2]

[1] One issue is that the expression complexity and binding limits are only coarse approximations for amp-bind runtime performance. For example, we don't factor in the asymptotic complexity for functions like concat() which are highly dependent on the size of input data.

[2] Need to consider the fact that expression parse/evaluation is render-blocking by default in amp-list. Parse cost can be mitigated with use of amp-bind-macro since the expressions are likely very similar. Definitely feeling more mixed about that change in hindsight.

/cc @ericlindley-g @aghassemi

@aghassemi
Copy link
Contributor

Great write up. Few thoughts:
1- Can incremental and initial limits be the exact same 1000 or are you suggesting incremental be like 500? I expect initial >> incremental.
2- Is amp-bind limits per-doc or per-window? Thinking about the content-level infinite scroll which currently can load 2 more full amp docs. Ideally the newly loaded AMP doc is treated as a separate context, specially given reevaluation is limited to its scope.
3- Total binding makes sense to me as a formula of n * incremental + initial where n is some reasonable expected load more hits. 5? so based on numbers above 3500, is that reasonable?

@dreamofabear
Copy link
Author

  1. Yea, I was thinking ~100-250 incremental limit. I think users have a lower tolerance for interactivity latency after page load vs. on page load, so it should be significantly lower than initial.
  2. The Bind service is scoped per-doc and scans the document body, so it should work as expected.
  3. Exactly, I was thinking ~2000-3000.

@dreamofabear
Copy link
Author

Similar request: #17233

@ampprojectbot
Copy link
Member

This issue hasn't been updated in awhile. @choumx Do you have any updates?

@ampprojectbot
Copy link
Member

This issue hasn't been updated in awhile. @choumx Do you have any updates?

@stale
Copy link

stale bot commented Dec 19, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale Inactive for one year or more label Dec 19, 2021
@stale stale bot closed this as completed Dec 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants