Skip to content

Commit 9cc0a2b

Browse files
authored
improve speed of generator (#23272)
* improve speed of generator * implement suggested fixes from CR. fix some general nitpicks. * implement suggested fixes from CR. fix some general nitpicks. * implement suggested fixes from CR. fix some general nitpicks. * revert some changes * fix * Revert "fix" This reverts commit 607c678. * Revert "revert some changes" This reverts commit 3b8e4c2. * fix
1 parent 7ad8951 commit 9cc0a2b

File tree

6 files changed

+312
-210
lines changed

6 files changed

+312
-210
lines changed

modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java

Lines changed: 175 additions & 169 deletions
Large diffs are not rendered by default.

modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultGenerator.java

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -86,16 +86,17 @@ public class DefaultGenerator implements Generator {
8686
private String basePath;
8787
private String basePathWithoutHost;
8888
private String contextPath;
89-
private Map<String, String> generatorPropertyDefaults = new HashMap<>();
89+
private final Map<String, String> generatorPropertyDefaults = new HashMap<>();
9090
/**
9191
* Retrieves an instance to the configured template processor, available after user-defined options are
9292
* applied via
9393
*/
94-
@Getter protected TemplateProcessor templateProcessor = null;
94+
@Getter
95+
protected TemplateProcessor templateProcessor = null;
9596

9697
private List<TemplateDefinition> userDefinedTemplates = new ArrayList<>();
97-
private String generatorCheck = "spring";
98-
private String templateCheck = "apiController.mustache";
98+
private final String generatorCheck = "spring";
99+
private final String templateCheck = "apiController.mustache";
99100

100101

101102
public DefaultGenerator() {
@@ -266,8 +267,7 @@ void configureGeneratorProperties() {
266267
openapiNormalizer.normalize();
267268
}
268269
} catch (Exception e) {
269-
LOGGER.error("An exception occurred in OpenAPI Normalizer. Please report the issue via https://github.com/openapitools/openapi-generator/issues/new/: ");
270-
e.printStackTrace();
270+
LOGGER.error("An exception occurred in OpenAPI Normalizer. Please report the issue via https://github.com/openapitools/openapi-generator/issues/new/: ", e);
271271
}
272272

273273
// resolve inline models
@@ -607,10 +607,10 @@ private void generateModelsForVariable(List<File> files, List<ModelMap> allModel
607607
if (!processedModels.contains(key) && allSchemas.containsKey(key)) {
608608
generateModels(files, allModels, unusedModels, aliasModels, processedModels, () -> Set.of(key));
609609
} else {
610-
LOGGER.info("Type " + variable.getComplexType() + " of variable " + variable.getName() + " could not be resolve because it is not declared as a model.");
610+
LOGGER.info("Type {} of variable {} could not be resolve because it is not declared as a model.", variable.getComplexType(), variable.getName());
611611
}
612612
} else {
613-
LOGGER.info("Type " + variable.getOpenApiType() + " of variable " + variable.getName() + " could not be resolve because it is not declared as a model.");
613+
LOGGER.info("Type {} of variable {} could not be resolve because it is not declared as a model.", variable.getOpenApiType(), variable.getName());
614614
}
615615
}
616616

@@ -639,8 +639,8 @@ private Set<String> getPropertyAsSet(String propertyName) {
639639
}
640640

641641
return Arrays.stream(propertyRaw.split(","))
642-
.map(String::trim)
643-
.collect(Collectors.toSet());
642+
.map(String::trim)
643+
.collect(Collectors.toSet());
644644
}
645645

646646
private Set<String> modelKeys() {
@@ -665,7 +665,6 @@ private Set<String> modelKeys() {
665665
return modelKeys;
666666
}
667667

668-
@SuppressWarnings("unchecked")
669668
void generateApis(List<File> files, List<OperationsMap> allOperations, List<ModelMap> allModels) {
670669
if (!generateApis) {
671670
// TODO: Process these anyway and present info via dryRun?
@@ -1006,7 +1005,7 @@ private void generateOpenapiGeneratorIgnoreFile() {
10061005
File ignoreFile = new File(ignoreFileNameTarget);
10071006
// use the entries provided by the users to pre-populate .openapi-generator-ignore
10081007
try {
1009-
LOGGER.info("Writing file " + ignoreFileNameTarget + " (which is always overwritten when the option `openapiGeneratorIgnoreFile` is enabled.)");
1008+
LOGGER.info("Writing file {} (which is always overwritten when the option `openapiGeneratorIgnoreFile` is enabled.)", ignoreFileNameTarget);
10101009
new File(config.outputFolder()).mkdirs();
10111010
if (!ignoreFile.createNewFile()) {
10121011
// file may already exist, do nothing
@@ -1430,7 +1429,10 @@ protected File processTemplateToFile(Map<String, Object> templateData, String te
14301429
return processTemplateToFile(templateData, templateName, outputFilename, shouldGenerate, skippedByOption, this.config.getOutputDir());
14311430
}
14321431

1433-
private final Set<String> seenFiles = new HashSet<>();
1432+
/**
1433+
* Stores lowercased absolute paths for O(1) case-insensitive duplicate detection.
1434+
*/
1435+
private final Set<String> seenFilesLower = new HashSet<>();
14341436

14351437
private File processTemplateToFile(Map<String, Object> templateData, String templateName, String outputFilename, boolean shouldGenerate, String skippedByOption, String intendedOutputDir) throws IOException {
14361438
String adjustedOutputFilename = outputFilename.replaceAll("//", "/").replace('/', File.separatorChar);
@@ -1443,10 +1445,10 @@ private File processTemplateToFile(Map<String, Object> templateData, String temp
14431445
throw new RuntimeException(String.format(Locale.ROOT, "Target files must be generated within the output directory; absoluteTarget=%s outDir=%s", absoluteTarget, outDir));
14441446
}
14451447

1446-
if (seenFiles.stream().filter(f -> f.toLowerCase(Locale.ROOT).equals(absoluteTarget.toString().toLowerCase(Locale.ROOT))).findAny().isPresent()) {
1447-
LOGGER.warn("Duplicate file path detected. Not all operating systems can handle case sensitive file paths. path={}", absoluteTarget.toString());
1448+
// O(1) case-insensitive duplicate check via a pre-lowercased shadow set
1449+
if (!seenFilesLower.add(absoluteTarget.toString().toLowerCase(Locale.ROOT))) {
1450+
LOGGER.warn("Duplicate file path detected. Not all operating systems can handle case sensitive file paths. path={}", absoluteTarget);
14481451
}
1449-
seenFiles.add(absoluteTarget.toString());
14501452
return this.templateProcessor.write(templateData, templateName, target);
14511453
} else {
14521454
this.templateProcessor.skip(target.toPath(), String.format(Locale.ROOT, "Skipped by %s options supplied by user.", skippedByOption));
@@ -2002,10 +2004,8 @@ private void generateFilesMetadata(List<File> files) {
20022004
}
20032005
});
20042006

2005-
Collections.sort(relativePaths, (a, b) -> IOCase.SENSITIVE.checkCompareTo(a, b));
2006-
relativePaths.forEach(relativePath -> {
2007-
sb.append(relativePath).append(System.lineSeparator());
2008-
});
2007+
relativePaths.sort(IOCase.SENSITIVE::checkCompareTo);
2008+
relativePaths.forEach(relativePath -> sb.append(relativePath).append(System.lineSeparator()));
20092009

20102010
String targetFile = config.outputFolder() + File.separator + METADATA_DIR + File.separator + config.getFilesMetadataFilename();
20112011

modules/openapi-generator/src/main/java/org/openapitools/codegen/TemplateManager.java

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.util.Map;
2222
import java.util.Objects;
2323
import java.util.Scanner;
24+
import java.util.concurrent.ConcurrentHashMap;
2425
import java.util.regex.Pattern;
2526

2627
/**
@@ -33,6 +34,9 @@ public class TemplateManager implements TemplatingExecutor, TemplateProcessor {
3334

3435
private final Logger LOGGER = LoggerFactory.getLogger(TemplateManager.class);
3536

37+
/** Cache of resolved template path -> raw template content, populated on first read per run. */
38+
private final Map<String, String> templateContentCache = new ConcurrentHashMap<>();
39+
3640
/**
3741
* Constructs a new instance of a {@link TemplateManager}
3842
*
@@ -75,7 +79,8 @@ private String getFullTemplateFile(String name) {
7579
*/
7680
@Override
7781
public String getFullTemplateContents(String name) {
78-
return readTemplate(getFullTemplateFile(name));
82+
String fullPath = getFullTemplateFile(name);
83+
return templateContentCache.computeIfAbsent(fullPath, this::readTemplate);
7984
}
8085

8186
/**
@@ -89,15 +94,22 @@ public Path getFullTemplatePath(String name) {
8994
return Paths.get(getFullTemplateFile(name));
9095
}
9196

97+
/**
98+
* Pre-compiled pattern for replacing the OS file separator with '/' in classpath resource paths.
99+
* Only non-null on operating systems where {@link File#separator} is not already '/'.
100+
*/
101+
private static final Pattern FILE_SEP_PATTERN =
102+
"/".equals(File.separator) ? null : Pattern.compile(Pattern.quote(File.separator));
103+
92104
/**
93105
* Gets a normalized classpath resource location according to OS-specific file separator
94106
*
95107
* @param name The name of the resource file/directory to find
96108
* @return A normalized string according to OS-specific file separator
97109
*/
98110
public static String getCPResourcePath(final String name) {
99-
if (!"/".equals(File.separator)) {
100-
return name.replaceAll(Pattern.quote(File.separator), "/");
111+
if (FILE_SEP_PATTERN != null) {
112+
return FILE_SEP_PATTERN.matcher(name).replaceAll("/");
101113
}
102114
return name;
103115
}
@@ -262,6 +274,8 @@ private File writeToFileRaw(String filename, byte[] contents) throws IOException
262274
}
263275

264276
private boolean filesEqual(File file1, File file2) throws IOException {
265-
return file1.exists() && file2.exists() && Arrays.equals(Files.readAllBytes(file1.toPath()), Files.readAllBytes(file2.toPath()));
277+
if (!file1.exists() || !file2.exists()) return false;
278+
if (file1.length() != file2.length()) return false;
279+
return Arrays.equals(Files.readAllBytes(file1.toPath()), Files.readAllBytes(file2.toPath()));
266280
}
267281
}

modules/openapi-generator/src/main/java/org/openapitools/codegen/templating/GeneratorTemplateContentLocator.java

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,22 @@
77

88
import java.io.File;
99
import java.nio.file.Paths;
10+
import java.util.Optional;
11+
import java.util.concurrent.ConcurrentHashMap;
1012

1113
/**
1214
* Locates templates according to {@link CodegenConfig} settings.
1315
*/
1416
public class GeneratorTemplateContentLocator implements TemplatePathLocator {
1517
private final CodegenConfig codegenConfig;
1618

19+
/**
20+
* Cache of relativeTemplateFile -> resolved full path (or empty Optional when the template does not exist).
21+
* The filesystem/classpath existence probes inside resolveFullTemplatePath are expensive on repeated calls
22+
* for the same template name, so we memoize the result for the lifetime of this locator instance.
23+
*/
24+
private final ConcurrentHashMap<String, Optional<String>> templatePathCache = new ConcurrentHashMap<>();
25+
1726
/**
1827
* Constructs a new instance of {@link GeneratorTemplateContentLocator} for the provided {@link CodegenConfig}
1928
*
@@ -51,12 +60,25 @@ private boolean classpathTemplateExists(String name) {
5160
* 4) (embedded template dir)
5261
* <p>
5362
* Where "template dir" may be user defined and "embedded template dir" are the built-in templates for the given generator.
63+
* <p>
64+
* Results are cached per {@code relativeTemplateFile} name because the filesystem/classpath probes are expensive
65+
* and the outcome is constant for the lifetime of this locator instance.
5466
*
5567
* @param relativeTemplateFile Template file
56-
* @return String Full template file path
68+
* @return String Full template file path, or {@code null} if the template does not exist in any location
5769
*/
5870
@Override
5971
public String getFullTemplatePath(String relativeTemplateFile) {
72+
return templatePathCache
73+
.computeIfAbsent(relativeTemplateFile, key -> Optional.ofNullable(resolveFullTemplatePath(key)))
74+
.orElse(null);
75+
}
76+
77+
/**
78+
* Performs the actual filesystem/classpath probes to find the full template path.
79+
* Called at most once per unique {@code relativeTemplateFile} value; all subsequent lookups use the cache.
80+
*/
81+
private String resolveFullTemplatePath(String relativeTemplateFile) {
6082
CodegenConfig config = this.codegenConfig;
6183

6284
//check the supplied template library folder for the file

modules/openapi-generator/src/main/java/org/openapitools/codegen/templating/HandlebarsEngineAdapter.java

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@
3838

3939
import java.io.IOException;
4040
import java.util.Arrays;
41-
import java.util.Locale;
4241
import java.util.Map;
42+
import java.util.concurrent.ConcurrentHashMap;
4343

4444
public class HandlebarsEngineAdapter extends AbstractTemplatingEngineAdapter {
4545
final Logger LOGGER = LoggerFactory.getLogger(HandlebarsEngineAdapter.class);
@@ -48,7 +48,24 @@ public class HandlebarsEngineAdapter extends AbstractTemplatingEngineAdapter {
4848
// We use this as a simple lookup for valid file name extensions. This adapter will inspect .mustache (built-in) and infer the relevant handlebars filename
4949
private final String[] canCompileFromExtensions = {".handlebars", ".hbs", ".mustache"};
5050
private boolean infiniteLoops = false;
51-
@Setter private boolean prettyPrint = false;
51+
@Setter
52+
private boolean prettyPrint = false;
53+
54+
/**
55+
* Per-executor cache of fully-configured {@link Handlebars} engine instances.
56+
* Each executor gets its own engine because the engine's {@link TemplateLoader} closes over the
57+
* executor; sharing an engine across executors would silently resolve templates from the wrong source.
58+
* {@link ConcurrentHashMap#computeIfAbsent} ensures the engine is built at most once per executor.
59+
*/
60+
private final ConcurrentHashMap<TemplatingExecutor, Handlebars> engineCache = new ConcurrentHashMap<>();
61+
62+
/**
63+
* Per-executor cache of compiled {@link Template} objects.
64+
* Keying on the executor instance eliminates the non-atomic check-clear-update invalidation
65+
* that the previous single-cache approach required; no state ever needs to be cleared.
66+
*/
67+
private final ConcurrentHashMap<TemplatingExecutor, ConcurrentHashMap<String, Template>> templateCaches =
68+
new ConcurrentHashMap<>();
5269

5370
/**
5471
* Provides an identifier used to load the adapter. This could be a name, uuid, or any other string.
@@ -63,13 +80,6 @@ public String getIdentifier() {
6380
@Override
6481
public String compileTemplate(TemplatingExecutor executor,
6582
Map<String, Object> bundle, String templateFile) throws IOException {
66-
TemplateLoader loader = new AbstractTemplateLoader() {
67-
@Override
68-
public TemplateSource sourceAt(String location) {
69-
return findTemplate(executor, location);
70-
}
71-
};
72-
7383
Context context = Context
7484
.newBuilder(bundle)
7585
.resolver(
@@ -79,9 +89,33 @@ public TemplateSource sourceAt(String location) {
7989
AccessAwareFieldValueResolver.INSTANCE)
8090
.build();
8191

92+
// Each executor gets its own Handlebars engine (the loader closes over the executor) and its
93+
// own compiled-template cache. computeIfAbsent is atomic, so concurrent calls with the same
94+
// executor share one engine/cache rather than racing to create duplicates.
95+
Handlebars handlebars = engineCache.computeIfAbsent(executor, this::buildHandlebars);
96+
ConcurrentHashMap<String, Template> cache =
97+
templateCaches.computeIfAbsent(executor, k -> new ConcurrentHashMap<>());
98+
99+
// Manual get → compile → put so IOException propagates naturally.
100+
Template tmpl = cache.get(templateFile);
101+
if (tmpl == null) {
102+
tmpl = handlebars.compile(templateFile);
103+
cache.put(templateFile, tmpl);
104+
}
105+
return tmpl.apply(context);
106+
}
107+
108+
/** Constructs and fully configures a {@link Handlebars} engine for the given executor. */
109+
private Handlebars buildHandlebars(TemplatingExecutor executor) {
110+
TemplateLoader loader = new AbstractTemplateLoader() {
111+
@Override
112+
public TemplateSource sourceAt(String location) {
113+
return findTemplate(executor, location);
114+
}
115+
};
82116
Handlebars handlebars = new Handlebars(loader);
83117
handlebars.registerHelperMissing((obj, options) -> {
84-
LOGGER.warn(String.format(Locale.ROOT, "Unregistered helper name '%s', processing template:%n%s", options.helperName, options.fn.text()));
118+
LOGGER.warn("Unregistered helper name '{}', processing template:\n{}", options.helperName, options.fn.text());
85119
return "";
86120
});
87121
handlebars.registerHelper("json", Jackson2Helper.INSTANCE);
@@ -90,8 +124,7 @@ public TemplateSource sourceAt(String location) {
90124
handlebars.registerHelpers(org.openapitools.codegen.templating.handlebars.StringHelpers.class);
91125
handlebars.setInfiniteLoops(infiniteLoops);
92126
handlebars.setPrettyPrint(prettyPrint);
93-
Template tmpl = handlebars.compile(templateFile);
94-
return tmpl.apply(context);
127+
return handlebars;
95128
}
96129

97130
@SuppressWarnings("java:S108")

modules/openapi-generator/src/main/java/org/openapitools/codegen/templating/MustacheEngineAdapter.java

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import java.io.StringReader;
3232
import java.io.StringWriter;
3333
import java.util.Map;
34+
import java.util.concurrent.ConcurrentHashMap;
3435

3536

3637
public class MustacheEngineAdapter implements TemplatingEngineAdapter {
@@ -51,6 +52,20 @@ public String getIdentifier() {
5152
@Getter @Setter
5253
Mustache.Compiler compiler = Mustache.compiler();
5354

55+
/**
56+
* Per-executor cache of template file name → compiled {@link Template}.
57+
* <p>
58+
* Keying on the executor instance eliminates the non-atomic check-clear-update invalidation pattern
59+
* that the previous single-cache approach required. Each executor gets its own independent inner
60+
* map, so different executors (e.g. different generator runs, test fixtures) can never observe
61+
* each other's compiled templates, and no state ever needs to be cleared.
62+
* <p>
63+
* {@link ConcurrentHashMap#computeIfAbsent} guarantees that the inner map for a given executor
64+
* is created exactly once even under concurrent access.
65+
*/
66+
private final ConcurrentHashMap<TemplatingExecutor, ConcurrentHashMap<String, Template>> compiledTemplateCaches =
67+
new ConcurrentHashMap<>();
68+
5469
/**
5570
* Compiles a template into a string
5671
*
@@ -62,10 +77,22 @@ public String getIdentifier() {
6277
*/
6378
@Override
6479
public String compileTemplate(TemplatingExecutor executor, Map<String, Object> bundle, String templateFile) throws IOException {
65-
Template tmpl = compiler
66-
.withLoader(name -> findTemplate(executor, name))
67-
.defaultValue("")
68-
.compile(executor.getFullTemplateContents(templateFile));
80+
// Each executor gets its own compiled-template cache. computeIfAbsent is atomic, so two threads
81+
// racing on the same executor key will share one inner map rather than creating two separate ones.
82+
ConcurrentHashMap<String, Template> cache =
83+
compiledTemplateCaches.computeIfAbsent(executor, k -> new ConcurrentHashMap<>());
84+
85+
// Manual get → compile → put so IOException propagates naturally.
86+
// At worst, two threads compile the same template simultaneously; the last writer wins,
87+
// which is harmless because compilation is pure/deterministic.
88+
Template tmpl = cache.get(templateFile);
89+
if (tmpl == null) {
90+
tmpl = compiler
91+
.withLoader(name -> findTemplate(executor, name))
92+
.defaultValue("")
93+
.compile(executor.getFullTemplateContents(templateFile));
94+
cache.put(templateFile, tmpl);
95+
}
6996
StringWriter out = new StringWriter();
7097

7198
// the value of bundle[MUSTACHE_PARENT_CONTEXT] is used a parent content in mustache.

0 commit comments

Comments
 (0)