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

perf(kubernetes): Reduce memory allocation during caching cycles #3736

Merged
merged 3 commits into from
Jun 3, 2019
Merged

perf(kubernetes): Reduce memory allocation during caching cycles #3736

merged 3 commits into from
Jun 3, 2019

Conversation

ezimanyi
Copy link
Contributor

@ezimanyi ezimanyi commented Jun 1, 2019

  • test(kubernetes): Add tests to getClusterRelationships

    Also simplify the logic in the invertRelationships test

  • perf(kubernetes): Reduce memory allocation during caching cycles

    The caching logic for the kubernetes v2 provider allocates a lot of short-term memory during each caching cycle, putting pressure on the garbage collector, in some cases exceeding the garbage collection overhead.

    A significant contributor is the logic to compute relationships between kubernetes objects. We currently create a CacheData object to hold each relationship and rely on downstream (and inefficient) logic to merge these into one CacheData per object, containing all of its relationships.

    Improve this by having invertRelationships return one CacheData per object (containing all of its relationships) rather than on CacheData per relationship. Likewise, have getClusterRelationships return a single CacheData for each application, rather than a separate one for each
    application-object relationship.

Also simplify the logic in the invertRelationships test
The caching logic for the kubernetes v2 provider allocates a
lot of short-term memory during each caching cycle, putting
pressure on the garbage collector, in some cases exceeding
the garbage collection overhead.

A significant contributor is the logic to compute relationships
between kubernetes objects. We currently create a CacheData object
to hold each relationship and rely on downstream (and inefficient)
logic to merge these into one CacheData per object, containing all
of its relationships.

Improve this by having invertRelationships return one CacheData
per object (containing all of its relationships) rather than on
CacheData per relationship. Likewise, have getClusterRelationships
return a single CacheData for each application, rather than a
separate one for each application-object relationship.
@ezimanyi
Copy link
Contributor Author

ezimanyi commented Jun 1, 2019

This should help significantly with spinnaker/spinnaker#3287.

Copy link
Contributor

@maggieneterval maggieneterval left a comment

Choose a reason for hiding this comment

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

🥇 💯 🎉 🙌 😃

@ezimanyi ezimanyi merged commit 00d249a into spinnaker:master Jun 3, 2019
@ezimanyi ezimanyi deleted the cache-perf branch June 3, 2019 17:19
@ezimanyi
Copy link
Contributor Author

ezimanyi commented Jun 4, 2019

@spinnakerbot cherry-pick 1.14

spinnakerbot pushed a commit that referenced this pull request Jun 4, 2019
* perf(kubernetes): Reduce memory allocation during caching cycles

The caching logic for the kubernetes v2 provider allocates a
lot of short-term memory during each caching cycle, putting
pressure on the garbage collector, in some cases exceeding
the garbage collection overhead.

A significant contributor is the logic to compute relationships
between kubernetes objects. We currently create a CacheData object
to hold each relationship and rely on downstream (and inefficient)
logic to merge these into one CacheData per object, containing all
of its relationships.

Improve this by having invertRelationships return one CacheData
per object (containing all of its relationships) rather than on
CacheData per relationship. Likewise, have getClusterRelationships
return a single CacheData for each application, rather than a
separate one for each application-object relationship.

* test(kubernetes): Add tests to getClusterRelationships

Also simplify the logic in the invertRelationships test
@spinnakerbot
Copy link
Contributor

Cherry pick successful: #3744

ezimanyi pushed a commit that referenced this pull request Jun 4, 2019
…) (#3744)

* perf(kubernetes): Reduce memory allocation during caching cycles

The caching logic for the kubernetes v2 provider allocates a
lot of short-term memory during each caching cycle, putting
pressure on the garbage collector, in some cases exceeding
the garbage collection overhead.

A significant contributor is the logic to compute relationships
between kubernetes objects. We currently create a CacheData object
to hold each relationship and rely on downstream (and inefficient)
logic to merge these into one CacheData per object, containing all
of its relationships.

Improve this by having invertRelationships return one CacheData
per object (containing all of its relationships) rather than on
CacheData per relationship. Likewise, have getClusterRelationships
return a single CacheData for each application, rather than a
separate one for each application-object relationship.

* test(kubernetes): Add tests to getClusterRelationships

Also simplify the logic in the invertRelationships test
maggieneterval pushed a commit that referenced this pull request Jun 4, 2019
…) (#3746)

* perf(kubernetes): Reduce memory allocation during caching cycles

The caching logic for the kubernetes v2 provider allocates a
lot of short-term memory during each caching cycle, putting
pressure on the garbage collector, in some cases exceeding
the garbage collection overhead.

A significant contributor is the logic to compute relationships
between kubernetes objects. We currently create a CacheData object
to hold each relationship and rely on downstream (and inefficient)
logic to merge these into one CacheData per object, containing all
of its relationships.

Improve this by having invertRelationships return one CacheData
per object (containing all of its relationships) rather than on
CacheData per relationship. Likewise, have getClusterRelationships
return a single CacheData for each application, rather than a
separate one for each application-object relationship.

* test(kubernetes): Add tests to getClusterRelationships

Also simplify the logic in the invertRelationships test
benjaminws added a commit to DataDog/clouddriver that referenced this pull request Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants