Skip to content

Commit

Permalink
perf(kubernetes): Store Kind on the KubernetesManifest object (#3853)
Browse files Browse the repository at this point in the history
* perf(kubernetes): Use null instead of optional

We create a lot of ScopedKind objects to look up kinds, so the
cost of creating an Optional each time is non-negligible.

In general using Optional can be more readable than null but in this
case we'd rather the performance of null (and it's just as easy to read
here).

* perf(kubernetes): Store Kind on the KubernetesManifest object

In processing manifests, we call getKind() multiple times per
manifest. It's not a cheap accessor as it needs to look up kinds
in the kind registry.

While recent changes have made looking up kinds a lot more efficient,
it still doesn't make sense to re-do this work multiple times per
manifest.

Compute the kind on-demand and store it on the object; clear the
cached value if we change either the kind or api group on the manifest.
  • Loading branch information
ezimanyi authored Jul 8, 2019
1 parent 3846e01 commit 8e5780e
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ public static class ScopedKind {
public ScopedKind(@Nonnull String name, @Nullable KubernetesApiGroup apiGroup) {
this.name = name;
this.lcName = name.toLowerCase();
this.apiGroup = Optional.ofNullable(apiGroup).orElse(KubernetesApiGroup.NONE);
this.apiGroup = apiGroup == null ? KubernetesApiGroup.NONE : apiGroup;
if (this.apiGroup.isNativeGroup()) {
this.customApiGroup = null;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.util.Map;
import java.util.Optional;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import lombok.Data;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.tuple.ImmutablePair;
Expand All @@ -34,6 +35,8 @@
public class KubernetesManifest extends HashMap<String, Object> {
private static ObjectMapper mapper = new ObjectMapper();

@Nullable private KubernetesKind computedKind;

@Override
public KubernetesManifest clone() {
return (KubernetesManifest) super.clone();
Expand All @@ -51,6 +54,14 @@ private static <T> T getRequiredField(KubernetesManifest manifest, String field)
@JsonIgnore
@Nonnull
public KubernetesKind getKind() {
if (computedKind == null) {
computedKind = computeKind();
}
return computedKind;
}

@Nonnull
private KubernetesKind computeKind() {
// using ApiVersion here allows a translation from a kind of NetworkPolicy in the manifest to
// something
// like NetworkPolicy.crd.projectcalico.org for custom resources
Expand All @@ -72,6 +83,7 @@ public String getKindName() {
@JsonIgnore
public void setKind(KubernetesKind kind) {
put("kind", kind.toString());
computedKind = null;
}

@JsonIgnore
Expand All @@ -82,6 +94,7 @@ public KubernetesApiVersion getApiVersion() {
@JsonIgnore
public void setApiVersion(KubernetesApiVersion apiVersion) {
put("apiVersion", apiVersion.toString());
computedKind = null;
}

@JsonIgnore
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,4 +129,34 @@ class KubernetesManifestSpec extends Specification {
then:
manifest.isNewerThanObservedGeneration()
}

void "correctly handles a change to the manifest's kind"() {
when:
def testPayload = gsonObj.fromJson(basicManifestSource(), Object)
KubernetesManifest manifest = objectToManifest(testPayload)

then:
manifest.getKind() == KIND

when:
manifest.setKind(KubernetesKind.DEPLOYMENT)

then:
manifest.getKind() == KubernetesKind.DEPLOYMENT
}

void "correctly handles a change to the manifest's API group"() {
when:
def testPayload = gsonObj.fromJson(basicManifestSource(), Object)
KubernetesManifest manifest = objectToManifest(testPayload)

then:
manifest.getApiVersion() == KubernetesApiVersion.EXTENSIONS_V1BETA1

when:
manifest.setApiVersion(KubernetesApiVersion.NETWORKING_K8S_IO_V1)

then:
manifest.getApiVersion() == KubernetesApiVersion.NETWORKING_K8S_IO_V1
}
}

0 comments on commit 8e5780e

Please sign in to comment.