-
Notifications
You must be signed in to change notification settings - Fork 92
parallelize generation and notification paths with Promise.all #1687
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -596,9 +596,13 @@ async function loadTemplates( | |
| templateData: [] | ||
| } | ||
| if (genTemplatesJsonArray != null && genTemplatesJsonArray.length > 0) { | ||
| for (let jsonFile of genTemplatesJsonArray) { | ||
| if (jsonFile == null || jsonFile == '') continue | ||
| let ctx = await loadGenTemplatesJsonFile(db, jsonFile) | ||
| const filesToLoad = genTemplatesJsonArray.filter( | ||
| (f) => f != null && f !== '' | ||
| ) | ||
| const loadResults = await Promise.all( | ||
| filesToLoad.map((jsonFile) => loadGenTemplatesJsonFile(db, jsonFile)) | ||
| ) | ||
|
Comment on lines
+602
to
+604
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While |
||
| for (const ctx of loadResults) { | ||
| if (ctx.error) { | ||
| if (options.failOnLoadingError) globalCtx.error = ctx.error | ||
| } else { | ||
|
|
@@ -761,28 +765,32 @@ async function generateAllTemplates( | |
| hb: hb | ||
| } | ||
|
|
||
| for (let pkg of packages) { | ||
| let outputOptions = await queryPackage.selectAllOptionsValues( | ||
| genResult.db, | ||
| pkg.id, | ||
| dbEnum.packageOptionCategory.outputOptions | ||
| ) | ||
| outputOptions.forEach((opt) => { | ||
| if (opt.optionCode == 'iterator') { | ||
| pkg.iterator = opt.optionLabel | ||
| } | ||
| }) | ||
| let generatorOptions = await queryPackage.selectAllOptionsValues( | ||
| genResult.db, | ||
| pkg.id, | ||
| dbEnum.packageOptionCategory.generator | ||
| ) | ||
| generatorOptions.forEach((opt) => { | ||
| if (opt.optionCode == 'static') { | ||
| pkg.static = opt.optionLabel | ||
| } | ||
| await Promise.all( | ||
| packages.map(async (pkg) => { | ||
| const [outputOptions, generatorOptions] = await Promise.all([ | ||
| queryPackage.selectAllOptionsValues( | ||
| genResult.db, | ||
| pkg.id, | ||
| dbEnum.packageOptionCategory.outputOptions | ||
| ), | ||
| queryPackage.selectAllOptionsValues( | ||
| genResult.db, | ||
| pkg.id, | ||
| dbEnum.packageOptionCategory.generator | ||
| ) | ||
| ]) | ||
| outputOptions.forEach((opt) => { | ||
| if (opt.optionCode == 'iterator') { | ||
| pkg.iterator = opt.optionLabel | ||
| } | ||
| }) | ||
| generatorOptions.forEach((opt) => { | ||
| if (opt.optionCode == 'static') { | ||
| pkg.static = opt.optionLabel | ||
| } | ||
| }) | ||
| }) | ||
| } | ||
| ) | ||
| // First extract overridePath if one exists, as we need to | ||
| // pass it to the generation. | ||
| packages.forEach((singlePkg) => { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parallelizing
setWarningIfMessageNotExistsintroduces a race condition. Since this function performs a "check-then-insert" operation without a database lock or transaction, concurrent calls for the same message (or duplicate messages in the array) can result in multiple identical notifications being created. It is safer to process these sequentially to maintain the uniqueness check.