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

Filter : Sanitise context used to evaluate Filter.enabled #5510

Merged
merged 1 commit into from
Oct 25, 2023

Conversation

johnhaddon
Copy link
Member

Without this, any input connections were evaluated once per location rather than just once, leading to much unnecessary pressure on the hash cache. It also provided a loophole via which you could modify the filter per location in a way that meant that ancestor and descendant matches would be inaccurate.

@johnhaddon johnhaddon self-assigned this Oct 20, 2023
@johnhaddon
Copy link
Member Author

Testing this with the scene where I originally noticed the problem, I'm seeing ~15% reduction in total hash count, and no appreciable change in total scene generation time.

@murraystevenson
Copy link
Contributor

and no appreciable change in total scene generation time.

I also did a bit of production testing at IE today and didn't see a measurable change in scene generation time, but it makes sense to close the loophole and any reduction in hash cache pressure is a good thing.

There's a wayward comma before "made" in the Changes.md entry, but otherwise looks good to me. 👍

Without this, any input connections were evaluated once per location rather than just once, leading to much unnecessary pressure on the hash cache. It also provided a loophole via which you could modify the filter per location in a way that meant that ancestor and descendant matches would be inaccurate.
@johnhaddon
Copy link
Member Author

There's a wayward comma before "made" in the Changes.md entry, but otherwise looks good to me.

Thanks - I've rounded up the stray, and am going to merge...

@johnhaddon johnhaddon merged commit 8220b28 into GafferHQ:1.3_maintenance Oct 25, 2023
4 checks passed
@johnhaddon johnhaddon deleted the filterContextLeak branch October 25, 2023 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants