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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add find filters to optimize where-first chains #8171

Merged
merged 3 commits into from May 21, 2020

Conversation

ashmaroli
Copy link
Member

@ashmaroli ashmaroli commented May 10, 2020

  • This is a 馃檵 feature or enhancement.
  • I've added tests.
  • I've adjusted the documentation.

Summary

Introducing Liquid filters find and find_exp to optimize scenarios involving first chained to where and where_exp filter respectively.

Purpose

where (where_exp) evaluates given conditions against all items in a given array and chaining the first filter returns a single object from the pool. This could be wasteful when the array is large with numerous items and the first winner is near the starting end.

i.e. say the array is ["apple", "lemonade", "spade", "sunglasses", "umbrella", "cola"]
{{ array | where: "type", "drink" | first }} and {{ array | find: "type", "drink" }} are equivalent and return the same object ("lemonade") but the latter stops processing after two items.

@DirtyF
Copy link
Member

DirtyF commented May 10, 2020

What's the expected gain?

If liquid don't have it, why should we?
https://shopify.github.io/liquid/filters/where/

@ashmaroli
Copy link
Member Author

ashmaroli commented May 10, 2020

Is that something that already exists in Liquid?

As of now, there is no find filter in upstream Liquid.

What's the expected gain?

For the general user-base? Depends on how it is used.
For Jekyll's docs site, profiles for #8172 should shed some light.

@ashmaroli
Copy link
Member Author

I've added a benchmark script to compare the performance of the find filter vs where + first filters to illustrate the possible boost.

@ashmaroli
Copy link
Member Author

ashmaroli commented May 18, 2020

If liquid don't have it, why should we?

@DirtyF Liquid has a different implementation of where filter from ours. So even if they were to add a find filter, it would be based on their version of where. That could have a different outcome for our users.

Copy link
Member

@DirtyF DirtyF left a comment

Choose a reason for hiding this comment

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

According to the profile docs tests, this brings a significant improvement for build time 馃憤

@DirtyF
Copy link
Member

DirtyF commented May 21, 2020

@jekyll: merge +minor

@jekyllbot jekyllbot merged commit 0b2c4c9 into jekyll:master May 21, 2020
jekyllbot added a commit that referenced this pull request May 21, 2020
@jekyll jekyll locked and limited conversation to collaborators May 21, 2021
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.

None yet

3 participants