Equality of schema.Sets gets tricky when dealing with nested sets -
Set.Equal only superficially compares the underlying maps and hence any
sets nested under the root sets cause issues.
This adds a simple method, HashEqual, that does a top-level hash
comparison, helping to work around this without any complex re-invention
of things like reflect.DeepEqual.
Of course, in order to make effective use of this function, the user
needs to make sure they are properly hashing their nested sets, however
this is trivial with things like HashResource.
Changing the Set internals makes a lot of sense as it saves doing
conversions in multiple places and gives a central place to alter
the key when a item is computed.
This will have no side effects other then that the ordering is now
based on strings instead on integers, so the order will be different.
This will however have no effect on existing configs as these will
use the individual codes/keys and not the ordering to determine if
there is a diff or not.
Lastly (but I think also most importantly) there is a fix in this PR
that makes diffing sets extremely more performand. Before a full diff
required reading the complete Set for every single parameter/attribute
you wanted to diff, while now it only gets that specific parameter.
We have a use case where we have a Set that has 18 parameters and the
set consist of about 600 items (don't ask 😉). So when doing a diff
it would take 100% CPU of all cores and stay that way for almost an
hour before being able to complete the diff.
Debugging this we learned that for retrieving every single parameter
it made over 52.000 calls to `func (c *ResourceConfig) get(..)`. In
this function a slice is created and used only for the duration of the
call, so the time needed to create all needed slices and on the other
hand the time the garbage collector needed to clean them up again caused
the system to cripple itself. Next to that there are also some expensive
reflect calls in this function which also claimed a fair amount of CPU
time.
After this fix the number of calls needed to get a single parameter
dropped from 52.000+ to only 2! 😃
Currently the `sync.Once` call is only used to init a Set in the add()
func. So when you add a value to a Set that is the result of one of the
Set operations (i.e. union, difference, intersect) the Set will be
reinitialised and the exiting values will be lost.
I don’t have a clue why this is showing up in my ACC tests just now, as
this code is in there for quite some time already. Somehow it seems to
have something to do with the refactoring of the helper/schema done
last week, as I cannot reproduce this with
47f02f80bc
Chris Marchesi