Skip to content

Fix for slow down with many setups on single mock#1111

Merged
stakx merged 5 commits intodevlooped:masterfrom
CeesKaas:master
Dec 28, 2020
Merged

Fix for slow down with many setups on single mock#1111
stakx merged 5 commits intodevlooped:masterfrom
CeesKaas:master

Conversation

@CeesKaas
Copy link
Contributor

@CeesKaas CeesKaas commented Nov 28, 2020

I've played with the code a bit this morning and this change fixes the unittests I've added (at the cost of a bit of extra memory consumption and an extra operation always to save n operations in most cases)

fixes #1110

@CeesKaas CeesKaas changed the title Attempted fix for #1110 Attempted fixes #1110 Nov 28, 2020
@CeesKaas CeesKaas changed the title Attempted fixes #1110 Attempted fix for slow down with many setups on single mock Nov 28, 2020
Copy link
Contributor

@stakx stakx left a comment

Choose a reason for hiding this comment

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

Hi @CeesKaas, thanks for your patience.

I do agree that we should do something about this problem. I've thought about it for a bit, and your solution indeed seems to be the easiest.

Just a couple of details that need your attention. Then this should be good to merge!

@stakx stakx added this to the 4.15.3 milestone Dec 28, 2020
CeesKaas and others added 2 commits December 28, 2020 22:05
Co-authored-by: stakx <stakx@eml.cc>
@CeesKaas CeesKaas requested a review from stakx December 28, 2020 21:13
lock (this.setups)
{
this.setups.Clear();
this.activeInvocationShapes.Clear();
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a remark for posteriority: in theory, we should also filter out removed setups in RemoveAllPropertyAccessorSetups. Not doing means that the first time a property setup is recreated after a call to mock.SetupAllProperties(), there may be a superfluous call to MarkOverriddenSetups. This is an edge case however, and the overhead of the additional MarkOverriddenSetups in this case is probably quite small, so let's just ignore this for now.

@stakx stakx changed the title Attempted fix for slow down with many setups on single mock Fix for slow down with many setups on single mock Dec 28, 2020
@stakx stakx merged commit 08d684c into devlooped:master Dec 28, 2020
@stakx
Copy link
Contributor

stakx commented Dec 28, 2020

All good. Thanks for contributing, @CeesKaas! 🚀

@stakx stakx modified the milestones: 4.15.3, 4.16.0 Jan 1, 2021
@devlooped devlooped locked and limited conversation to collaborators Sep 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adding a setup looks like it became an O(n*n) operation since #984 was merged

2 participants