Skip to content

Commit

Permalink
[core] Sanitize/underscore/camelize cache expiry (#5484)
Browse files Browse the repository at this point in the history 
The helper methods for sanitize/underscore/camelize were recently
updated to cache values in static maps. This would lead to a memory leak
in hosted environments as the maps had no cleanup/expiry.

This moves those cached entries to Caffeine caches with expiry based on
last access to allow the edge-case performance improvement gains on very
large documents, without the memory leak in hosted or embedded
environments.
  • Loading branch information
@jimschubert
jimschubert authored Mar 3, 2020
1 parent c27d400 commit 51cc7c2
Show file tree
Hide file tree
Showing 2 changed files with 193 additions and 140 deletions.
144 changes: 93 additions & 51 deletions modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@

package org.openapitools.codegen;

import com.github.benmanes.caffeine.cache.Cache;
import com.github.benmanes.caffeine.cache.Caffeine;
import com.github.benmanes.caffeine.cache.Ticker;
import com.google.common.base.CaseFormat;
import com.google.common.collect.ImmutableMap;
import com.samskivert.mustache.Mustache;
Expand Down Expand Up @@ -67,6 +70,7 @@
import java.io.File;
import java.util.*;
import java.util.Map.Entry;
import java.util.concurrent.TimeUnit;
import java.util.function.Consumer;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand All @@ -80,6 +84,10 @@ public class DefaultCodegen implements CodegenConfig {

public static FeatureSet DefaultFeatureSet;

// A cache of sanitized words. The sanitizeName() method is invoked many times with the same
// arguments, this cache is used to optimized performance.
private static Cache<SanitizeNameOptions, String> sanitizedNameCache;

static {
DefaultFeatureSet = FeatureSet.newBuilder()
.includeDataTypeFeatures(
Expand Down Expand Up @@ -123,6 +131,12 @@ public class DefaultCodegen implements CodegenConfig {
// PROTOBUF and Custom are generator specific
)
.build();

sanitizedNameCache = Caffeine.newBuilder()
.maximumSize(500)
.expireAfterAccess(10, TimeUnit.SECONDS)
.ticker(Ticker.systemTicker())
.build();
}

protected GeneratorMetadata generatorMetadata;
Expand Down Expand Up @@ -4505,21 +4519,16 @@ public String sanitizeName(String name, String removeCharRegEx) {
return sanitizeName(name, removeCharRegEx, new ArrayList<String>());
}

// A cache of sanitized words. The sanitizeName() method is invoked many times with the same
// arguments, this cache is used to optimized performance.
private static Map<String, Map<String, Map<List<String>, String>>> sanitizedNames =
new HashMap<String, Map<String, Map<List<String>, String>>>();

/**
* Sanitize name (parameter, property, method, etc)
*
* @param name string to be sanitize
* @param removeCharRegEx a regex containing all char that will be removed
* @param exceptionList a list of matches which should not be sanitized (i.e expections)
* @param exceptionList a list of matches which should not be sanitized (i.e exception)
* @return sanitized string
*/
@SuppressWarnings("static-method")
public String sanitizeName(String name, String removeCharRegEx, ArrayList<String> exceptionList) {
public String sanitizeName(final String name, String removeCharRegEx, ArrayList<String> exceptionList) {
// NOTE: performance wise, we should have written with 2 replaceAll to replace desired
// character with _ or empty character. Below aims to spell out different cases we've
// encountered so far and hopefully make it easier for others to add more special
Expand All @@ -4536,61 +4545,51 @@ public String sanitizeName(String name, String removeCharRegEx, ArrayList<String
return "value";
}

Map<String, Map<List<String>, String>> m1 = sanitizedNames.get(name);
if (m1 == null) {
m1 = new HashMap<String, Map<List<String>, String>>();
sanitizedNames.put(name, m1);
}
Map<List<String>, String> m2 = m1.get(removeCharRegEx);
if (m2 == null) {
m2 = new HashMap<List<String>, String>();
m1.put(removeCharRegEx, m2);
}
List<String> l = Collections.unmodifiableList(exceptionList);
if (m2.containsKey(l)) {
return m2.get(l);
}
SanitizeNameOptions opts = new SanitizeNameOptions(name, removeCharRegEx, exceptionList);

// input[] => input
name = this.sanitizeValue(name, "\\[\\]", "", exceptionList);
return sanitizedNameCache.get(opts, sanitizeNameOptions -> {
String modifiable = sanitizeNameOptions.getName();
List<String> exceptions = sanitizeNameOptions.getExceptions();
// input[] => input
modifiable = this.sanitizeValue(modifiable, "\\[\\]", "", exceptions);

// input[a][b] => input_a_b
name = this.sanitizeValue(name, "\\[", "_", exceptionList);
name = this.sanitizeValue(name, "\\]", "", exceptionList);
// input[a][b] => input_a_b
modifiable = this.sanitizeValue(modifiable, "\\[", "_", exceptions);
modifiable = this.sanitizeValue(modifiable, "\\]", "", exceptions);

// input(a)(b) => input_a_b
name = this.sanitizeValue(name, "\\(", "_", exceptionList);
name = this.sanitizeValue(name, "\\)", "", exceptionList);
// input(a)(b) => input_a_b
modifiable = this.sanitizeValue(modifiable, "\\(", "_", exceptions);
modifiable = this.sanitizeValue(modifiable, "\\)", "", exceptions);

// input.name => input_name
name = this.sanitizeValue(name, "\\.", "_", exceptionList);
// input.name => input_name
modifiable = this.sanitizeValue(modifiable, "\\.", "_", exceptions);

// input-name => input_name
name = this.sanitizeValue(name, "-", "_", exceptionList);
// input-name => input_name
modifiable = this.sanitizeValue(modifiable, "-", "_", exceptions);

// a|b => a_b
name = this.sanitizeValue(name, "\\|", "_", exceptionList);
// a|b => a_b
modifiable = this.sanitizeValue(modifiable, "\\|", "_", exceptions);

// input name and age => input_name_and_age
name = this.sanitizeValue(name, " ", "_", exceptionList);
// input name and age => input_name_and_age
modifiable = this.sanitizeValue(modifiable, " ", "_", exceptions);

// /api/films/get => _api_films_get
// \api\films\get => _api_films_get
name = name.replaceAll("/", "_");
name = name.replaceAll("\\\\", "_");
// /api/films/get => _api_films_get
// \api\films\get => _api_films_get
modifiable = modifiable.replaceAll("/", "_");
modifiable = modifiable.replaceAll("\\\\", "_");

// remove everything else other than word, number and _
// $php_variable => php_variable
if (allowUnicodeIdentifiers) { //could be converted to a single line with ?: operator
name = Pattern.compile(removeCharRegEx, Pattern.UNICODE_CHARACTER_CLASS).matcher(name).replaceAll("");
} else {
name = name.replaceAll(removeCharRegEx, "");
}
m2.put(l, name);
return name;
// remove everything else other than word, number and _
// $php_variable => php_variable
if (allowUnicodeIdentifiers) { //could be converted to a single line with ?: operator
modifiable = Pattern.compile(sanitizeNameOptions.getRemoveCharRegEx(), Pattern.UNICODE_CHARACTER_CLASS).matcher(modifiable).replaceAll("");
} else {
modifiable = modifiable.replaceAll(sanitizeNameOptions.getRemoveCharRegEx(), "");
}
return modifiable;
});
}

private String sanitizeValue(String value, String replaceMatch, String replaceValue, ArrayList<String> exceptionList) {
private String sanitizeValue(String value, String replaceMatch, String replaceValue, List<String> exceptionList) {
if (exceptionList.size() == 0 || !exceptionList.contains(replaceMatch)) {
return value.replaceAll(replaceMatch, replaceValue);
}
Expand Down Expand Up @@ -5748,4 +5747,47 @@ protected void modifyFeatureSet(Consumer<FeatureSet.Builder> processor) {
this.generatorMetadata = GeneratorMetadata.newBuilder(generatorMetadata)
.featureSet(builder.build()).build();
}

private static class SanitizeNameOptions {
public SanitizeNameOptions(String name, String removeCharRegEx, List<String> exceptions) {
this.name = name;
this.removeCharRegEx = removeCharRegEx;
if (exceptions != null) {
this.exceptions = Collections.unmodifiableList(exceptions);
} else {
this.exceptions = Collections.unmodifiableList(new ArrayList<>());
}
}

public String getName() {
return name;
}

public String getRemoveCharRegEx() {
return removeCharRegEx;
}

public List<String> getExceptions() {
return exceptions;
}

private String name;
private String removeCharRegEx;
private List<String> exceptions;

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
SanitizeNameOptions that = (SanitizeNameOptions) o;
return Objects.equals(getName(), that.getName()) &&
Objects.equals(getRemoveCharRegEx(), that.getRemoveCharRegEx()) &&
Objects.equals(getExceptions(), that.getExceptions());
}

@Override
public int hashCode() {
return Objects.hash(getName(), getRemoveCharRegEx(), getExceptions());
}
}
}
Loading

0 comments on commit 51cc7c2

Please sign in to comment.