Skip to content

Conversation

@antepusic
Copy link
Contributor

@antepusic antepusic commented Aug 15, 2023

Evaluating expressions that contain many same-variable property lookups is slower than optimal because GetProperty has linear complexity. This PR attempts to solve that by preloading and caching all properties of the variables that have multiple property lookups; the properties are cached in the execution context.

Related to #1159 (SetProperty optimization).

[master < Task] PR

  • Check, and update documentation if necessary
  • Provide the full content or a guide for the final git message

Introduce property caching for multiple same-variable property lookups

To keep docs changelog up to date, one more thing to do:

  • Write a release note here, including added/changed clauses

We’ve optimized queries that build maps with multiple same-variable property lookups.

  • Tag someone from docs team in the comments

antepusic and others added 27 commits July 26, 2023 15:41
@antepusic antepusic self-assigned this Aug 15, 2023
@antepusic antepusic marked this pull request as draft August 15, 2023 22:48
Copy link
Contributor

@Josipmrden Josipmrden left a comment

Choose a reason for hiding this comment

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

Overall good job, but some files are present that should not be.

Copy link
Contributor

@andrejtonev andrejtonev left a comment

Choose a reason for hiding this comment

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

Looks good! 💪
Some C++ related comments.
Plus a few places we could maybe gain performance as well. Not sure about them, since I'm missing the big picture. If you want we could jump on a call Monday.

@antepusic antepusic linked an issue Aug 24, 2023 that may be closed by this pull request
@antepusic antepusic added the feature feature label Sep 6, 2023
Copy link
Contributor

@andrejtonev andrejtonev left a comment

Choose a reason for hiding this comment

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

Great job!

@antepusic
Copy link
Contributor Author

@vpavicic Release note in the original comment

@antepusic antepusic added Ship it PR ready to be merged and removed Ready for review PR is ready for review labels Sep 11, 2023
@Josipmrden Josipmrden merged commit 29a505c into master Sep 11, 2023
@Josipmrden Josipmrden deleted the property-lookup-caching branch September 11, 2023 11:03
@gitbuda gitbuda removed the Ship it PR ready to be merged label Sep 11, 2023
@vpavicic vpavicic added the Docs - changelog only Docs - changelog only label Sep 12, 2023
as51340 pushed a commit that referenced this pull request Nov 10, 2023
as51340 pushed a commit that referenced this pull request Oct 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Docs - changelog only Docs - changelog only feature feature

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

SetProperties working poorly when there is a lot of indexes

6 participants