From bee03685f06ba60cf4ef2677a9b15bbc730e42af Mon Sep 17 00:00:00 2001 From: Sebastien Rosset Date: Tue, 18 Aug 2020 11:40:49 -0700 Subject: [PATCH] [codegen][Python] Add model cache to speed up code generation (#7220) * Add modelName to schema cache. * Add modelName to schema cache. * add cache optimizations * add cache optimizations * remove unused variable --- .../openapitools/codegen/DefaultCodegen.java | 91 +++++++++++--- .../PythonClientExperimentalCodegen.java | 11 +- .../codegen/utils/StringUtils.java | 117 +++++++++++++----- 3 files changed, 169 insertions(+), 50 deletions(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java index 92a3f76b760..b29b2063868 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java @@ -246,6 +246,9 @@ public class DefaultCodegen implements CodegenConfig { // make openapi available to all methods protected OpenAPI openAPI; + // A cache to efficiently lookup a Schema instance based on the return value of `toModelName()`. + private Map modelNameToSchemaCache; + public List cliOptions() { return cliOptions; } @@ -449,6 +452,23 @@ public class DefaultCodegen implements CodegenConfig { return objs; } + /** + * Return a map from model name to Schema for efficient lookup. + * + * @return map from model name to Schema. + */ + protected Map getModelNameToSchemaCache() { + if (modelNameToSchemaCache == null) { + // Create a cache to efficiently lookup schema based on model name. + Map m = new HashMap(); + ModelUtils.getSchemas(openAPI).forEach((key, schema) -> { + m.put(toModelName(key), schema); + }); + modelNameToSchemaCache = Collections.unmodifiableMap(m); + } + return modelNameToSchemaCache; + } + /** * Index all CodegenModels by model name. * @@ -1292,14 +1312,13 @@ public class DefaultCodegen implements CodegenConfig { * @param name the variable name * @return the sanitized variable name */ - public String toVarName(String name) { + public String toVarName(final String name) { if (reservedWords.contains(name)) { return escapeReservedWord(name); } else if (((CharSequence) name).chars().anyMatch(character -> specialCharReplacements.keySet().contains("" + ((char) character)))) { return escape(name, specialCharReplacements, null, null); - } else { - return name; } + return name; } /** @@ -1309,14 +1328,15 @@ public class DefaultCodegen implements CodegenConfig { * @param name Codegen property object * @return the sanitized parameter name */ - public String toParamName(String name) { - name = removeNonNameElementToCamelCase(name); // FIXME: a parameter should not be assigned. Also declare the methods parameters as 'final'. - if (reservedWords.contains(name)) { - return escapeReservedWord(name); - } else if (((CharSequence) name).chars().anyMatch(character -> specialCharReplacements.keySet().contains("" + ((char) character)))) { - return escape(name, specialCharReplacements, null, null); + public String toParamName(final String name) { + String modifiable = removeNonNameElementToCamelCase(name); // FIXME: a parameter should not be assigned. Also declare the methods parameters as 'final'. + if (reservedWords.contains(modifiable)) { + return escapeReservedWord(modifiable); + } else if (((CharSequence) modifiable).chars().anyMatch(character -> specialCharReplacements.keySet().contains("" + ((char) character)))) { + return escape(modifiable, specialCharReplacements, null, null); } - return name; + return modifiable; + } /** @@ -2178,7 +2198,8 @@ public class DefaultCodegen implements CodegenConfig { } /** - * Output the proper model name (capitalized). + * Converts the OpenAPI schema name to a model name suitable for the current code generator. + * May be overriden for each programming language. * In case the name belongs to the TypeSystem it won't be renamed. * * @param name the name of the model @@ -2188,6 +2209,32 @@ public class DefaultCodegen implements CodegenConfig { return camelize(modelNamePrefix + "_" + name + "_" + modelNameSuffix); } + private static class NamedSchema { + private NamedSchema(String name, Schema s) { + this.name = name; + this.schema = s; + } + private String name; + private Schema schema; + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + NamedSchema that = (NamedSchema) o; + return Objects.equals(name, that.name) && + Objects.equals(schema, that.schema); + } + + @Override + public int hashCode() { + return Objects.hash(name, schema); + } + } + + Map schemaCodegenModelCache = new HashMap(); + Map schemaCodegenPropertyCache = new HashMap(); + /** * Convert OAS Model object to Codegen Model object * @@ -2196,6 +2243,13 @@ public class DefaultCodegen implements CodegenConfig { * @return Codegen Model object */ public CodegenModel fromModel(String name, Schema schema) { + NamedSchema ns = new NamedSchema(name, schema); + CodegenModel cm = schemaCodegenModelCache.get(ns); + if (cm != null) { + LOGGER.debug("Cached fromModel for " + name + " : " + schema.getName()); + return cm; + } + Map allDefinitions = ModelUtils.getSchemas(this.openAPI); if (typeAliases == null) { // Only do this once during first call @@ -2544,7 +2598,7 @@ public class DefaultCodegen implements CodegenConfig { postProcessModelProperty(m, prop); } } - + schemaCodegenModelCache.put(ns, m); return m; } @@ -2976,7 +3030,12 @@ public class DefaultCodegen implements CodegenConfig { return null; } LOGGER.debug("debugging fromProperty for " + name + " : " + p); - + NamedSchema ns = new NamedSchema(name, p); + CodegenProperty c = schemaCodegenPropertyCache.get(ns); + if (c != null) { + LOGGER.debug("Cached fromProperty for " + name + " : " + p.getName()); + return c; + } // unalias schema p = ModelUtils.unaliasSchema(this.openAPI, p, importMapping); @@ -3278,6 +3337,7 @@ public class DefaultCodegen implements CodegenConfig { } LOGGER.debug("debugging from property return: " + property); + schemaCodegenPropertyCache.put(ns, property); return property; } @@ -6356,6 +6416,9 @@ public class DefaultCodegen implements CodegenConfig { .featureSet(builder.build()).build(); } + /** + * An map entry for cached sanitized names. + */ private static class SanitizeNameOptions { public SanitizeNameOptions(String name, String removeCharRegEx, List exceptions) { this.name = name; @@ -6363,7 +6426,7 @@ public class DefaultCodegen implements CodegenConfig { if (exceptions != null) { this.exceptions = Collections.unmodifiableList(exceptions); } else { - this.exceptions = Collections.unmodifiableList(new ArrayList<>()); + this.exceptions = Collections.emptyList(); } } diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java index afe00619c0c..1d2f0bc5129 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java @@ -447,19 +447,16 @@ public class PythonClientExperimentalCodegen extends PythonClientCodegen { } String varDataType = var.mostInnerItems != null ? var.mostInnerItems.dataType : var.dataType; - Optional referencedSchema = ModelUtils.getSchemas(openAPI).entrySet().stream() - .filter(entry -> Objects.equals(varDataType, toModelName(entry.getKey()))) - .map(Map.Entry::getValue) - .findFirst(); - String dataType = (referencedSchema.isPresent()) ? getTypeDeclaration(referencedSchema.get()) : varDataType; + Schema referencedSchema = getModelNameToSchemaCache().get(varDataType); + String dataType = (referencedSchema != null) ? getTypeDeclaration(referencedSchema) : varDataType; // put "enumVars" map into `allowableValues", including `name` and `value` List> enumVars = buildEnumVars(values, dataType); // if "x-enum-varnames" or "x-enum-descriptions" defined, update varnames Map extensions = var.mostInnerItems != null ? var.mostInnerItems.getVendorExtensions() : var.getVendorExtensions(); - if (referencedSchema.isPresent()) { - extensions = referencedSchema.get().getExtensions(); + if (referencedSchema != null) { + extensions = referencedSchema.getExtensions(); } updateEnumVarsWithExtensions(enumVars, extensions); allowableValues.put("enumVars", enumVars); diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/StringUtils.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/StringUtils.java index 8f70d05b9e8..9bca83d638b 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/StringUtils.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/StringUtils.java @@ -7,10 +7,13 @@ import org.apache.commons.lang3.tuple.ImmutablePair; import org.apache.commons.lang3.tuple.Pair; import org.openapitools.codegen.config.GlobalSettings; +import java.util.Collections; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.HashMap; +import java.util.Objects; +import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -18,11 +21,11 @@ import java.util.regex.Pattern; public class StringUtils { /** - * Set the cache size (entry count) of the sanitizedNameCache, camelizedWordsCache and underscoreWords. + * Set the cache size (entry count) of the sanitizedNameCache, camelizedWordsCache and underscoreWordsCache. */ public static final String NAME_CACHE_SIZE_PROPERTY = "org.openapitools.codegen.utils.namecache.cachesize"; /** - * Set the cache expiry (in seconds) of the sanitizedNameCache, camelizedWordsCache and underscoreWords. + * Set the cache expiry (in seconds) of the sanitizedNameCache, camelizedWordsCache and underscoreWordsCache. */ public static final String NAME_CACHE_EXPIRY_PROPERTY = "org.openapitools.codegen.utils.namecache.expireafter.seconds"; @@ -31,7 +34,11 @@ public class StringUtils { private static Cache, String> camelizedWordsCache; // A cache of underscored words, used to optimize the performance of the underscore() method. - private static Cache underscoreWords; + private static Cache underscoreWordsCache; + + // A cache of escaped words, used to optimize the performance of the escape() method. + private static Cache escapedWordsCache; + static { int cacheSize = Integer.parseInt(GlobalSettings.getProperty(NAME_CACHE_SIZE_PROPERTY, "200")); int cacheExpiry = Integer.parseInt(GlobalSettings.getProperty(NAME_CACHE_EXPIRY_PROPERTY, "5")); @@ -41,13 +48,24 @@ public class StringUtils { .ticker(Ticker.systemTicker()) .build(); - underscoreWords = Caffeine.newBuilder() + escapedWordsCache = Caffeine.newBuilder() + .maximumSize(cacheSize) + .expireAfterAccess(cacheExpiry, TimeUnit.SECONDS) + .ticker(Ticker.systemTicker()) + .build(); + + underscoreWordsCache = Caffeine.newBuilder() .maximumSize(cacheSize) .expireAfterAccess(cacheExpiry, TimeUnit.SECONDS) .ticker(Ticker.systemTicker()) .build(); } + private static Pattern capitalLetterPattern = Pattern.compile("([A-Z]+)([A-Z][a-z])"); + private static Pattern lowercasePattern = Pattern.compile("([a-z\\d])([A-Z])"); + private static Pattern pkgSeparatorPattern = Pattern.compile("\\."); + private static Pattern dollarPattern = Pattern.compile("\\$"); + /** * Underscore the given word. * Copied from Twitter elephant bird @@ -57,18 +75,16 @@ public class StringUtils { * @return The underscored version of the word */ public static String underscore(final String word) { - return underscoreWords.get(word, wordToUnderscore -> { + return underscoreWordsCache.get(word, wordToUnderscore -> { String result; - String firstPattern = "([A-Z]+)([A-Z][a-z])"; - String secondPattern = "([a-z\\d])([A-Z])"; String replacementPattern = "$1_$2"; // Replace package separator with slash. - result = wordToUnderscore.replaceAll("\\.", "/"); + result = pkgSeparatorPattern.matcher(wordToUnderscore).replaceAll("/"); // Replace $ with two underscores for inner classes. - result = result.replaceAll("\\$", "__"); + result = dollarPattern.matcher(result).replaceAll("__"); // Replace capital letter with _ plus lowercase letter. - result = result.replaceAll(firstPattern, replacementPattern); - result = result.replaceAll(secondPattern, replacementPattern); + result = capitalLetterPattern.matcher(result).replaceAll(replacementPattern); + result = lowercasePattern.matcher(result).replaceAll(replacementPattern); result = result.replace('-', '_'); // replace space with underscore result = result.replace(' ', '_'); @@ -103,6 +119,8 @@ public class StringUtils { private static Pattern camelizeUppercasePattern = Pattern.compile("(\\.?)(\\w)([^\\.]*)$"); private static Pattern camelizeUnderscorePattern = Pattern.compile("(_)(.)"); private static Pattern camelizeHyphenPattern = Pattern.compile("(-)(.)"); + private static Pattern camelizeDollarPattern = Pattern.compile("\\$"); + private static Pattern camelizeSimpleUnderscorePattern = Pattern.compile("_"); /** * Camelize name (parameter, property, method, etc) @@ -144,7 +162,7 @@ public class StringUtils { m = camelizeUppercasePattern.matcher(word); if (m.find()) { String rep = m.group(1) + m.group(2).toUpperCase(Locale.ROOT) + m.group(3); - rep = rep.replaceAll("\\$", "\\\\\\$"); + rep = camelizeDollarPattern.matcher(rep).replaceAll("\\\\\\$"); word = m.replaceAll(rep); } @@ -154,7 +172,7 @@ public class StringUtils { String original = m.group(2); String upperCase = original.toUpperCase(Locale.ROOT); if (original.equals(upperCase)) { - word = word.replaceFirst("_", ""); + word = camelizeSimpleUnderscorePattern.matcher(word).replaceFirst(""); } else { word = m.replaceFirst(upperCase); } @@ -180,11 +198,49 @@ public class StringUtils { } // remove all underscore - word = word.replaceAll("_", ""); + word = camelizeSimpleUnderscorePattern.matcher(word).replaceAll(""); return word; }); } + private static class EscapedNameOptions { + public EscapedNameOptions(String name, Set specialChars, List charactersToAllow, String appendToReplacement) { + this.name = name; + this.appendToReplacement = appendToReplacement; + if (specialChars != null) { + this.specialChars = Collections.unmodifiableSet(specialChars); + } else { + this.specialChars = Collections.emptySet(); + } + if (charactersToAllow != null) { + this.charactersToAllow = Collections.unmodifiableList(charactersToAllow); + } else { + this.charactersToAllow = Collections.emptyList(); + } + } + + private String name; + private String appendToReplacement; + private Set specialChars; + private List charactersToAllow; + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + EscapedNameOptions that = (EscapedNameOptions) o; + return Objects.equals(name, that.name) && + Objects.equals(appendToReplacement, that.appendToReplacement) && + Objects.equals(specialChars, that.specialChars) && + Objects.equals(charactersToAllow, that.charactersToAllow); + } + + @Override + public int hashCode() { + return Objects.hash(name, appendToReplacement, specialChars, charactersToAllow); + } + } + /** * Return the name with escaped characters. * @@ -196,20 +252,23 @@ public class StringUtils { *

* throws Runtime exception as word is not escaped properly. */ - public static String escape(String name, Map replacementMap, - List charactersToAllow, String appendToReplacement) { - String result = name.chars().mapToObj(c -> { - String character = "" + (char) c; - if (charactersToAllow != null && charactersToAllow.contains(character)) { - return character; - } else if (replacementMap.containsKey(character)) { - return replacementMap.get(character) + (appendToReplacement != null ? appendToReplacement: ""); - } else { - return character; - } - }).reduce( (c1, c2) -> "" + c1 + c2).orElse(null); - - if (result != null) return result; - throw new RuntimeException("Word '" + name + "' could not be escaped."); + public static String escape(final String name, final Map replacementMap, + final List charactersToAllow, final String appendToReplacement) { + EscapedNameOptions ns = new EscapedNameOptions(name, replacementMap.keySet(), charactersToAllow, appendToReplacement); + return escapedWordsCache.get(ns, wordToEscape -> { + String result = name.chars().mapToObj(c -> { + String character = "" + (char) c; + if (charactersToAllow != null && charactersToAllow.contains(character)) { + return character; + } else if (replacementMap.containsKey(character)) { + return replacementMap.get(character) + (appendToReplacement != null ? appendToReplacement: ""); + } else { + return character; + } + }).reduce( (c1, c2) -> "" + c1 + c2).orElse(null); + + if (result != null) return result; + throw new RuntimeException("Word '" + name + "' could not be escaped."); + }); } }