Skip to content

Commit

Permalink
transformer workaround memory leak when processing large models (#3253)
Browse files Browse the repository at this point in the history
- see this issue report of the leak nodejs/node-addon-api#1140
- the leak appears to be a bug in node[-addon-api]. Tracing the gc shows JS heap size remains stable and old-generation garbage collection sweeps occur in parallel with the JS work, but objects that use `Napi::ObjectWrap` hold on to some memory until the thread yields. See the above issue for more details.
- the workaround is to force yield the javascript job in long loops
- the transformation service already employs this workaround to handle huge models

Co-authored-by: Michael Belousov <MichaelBelousov@users.noreply.github.com>
  • Loading branch information
MichaelBelousov and MichaelBelousov committed Feb 22, 2022
1 parent d698388 commit 6d9dba7
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 1 deletion.
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/core-transformer",
"comment": "mitigate memory leak in large model processing",
"type": "none"
}
],
"packageName": "@itwin/core-transformer"
}
2 changes: 2 additions & 0 deletions core/transformer/src/IModelExporter.ts
Expand Up @@ -475,6 +475,7 @@ export class IModelExporter {
}
while (DbResult.BE_SQLITE_ROW === statement.step()) {
await this.exportElement(statement.getValue(0).getId());
await new Promise(setImmediate); // workaround for: https://github.com/nodejs/node-addon-api/issues/1140
}
});
}
Expand Down Expand Up @@ -638,6 +639,7 @@ export class IModelExporter {
const relInstanceId: Id64String = statement.getValue(0).getId();
const relProps: RelationshipProps = this.sourceDb.relationships.getInstanceProps(baseRelClassFullName, relInstanceId);
await this.exportRelationship(relProps.classFullName, relInstanceId); // must call exportRelationship using the actual classFullName, not baseRelClassFullName
await new Promise(setImmediate); // workaround for: https://github.com/nodejs/node-addon-api/issues/1140
}
});
}
Expand Down
3 changes: 2 additions & 1 deletion core/transformer/src/IModelTransformer.ts
Expand Up @@ -709,7 +709,7 @@ export class IModelTransformer extends IModelExportHandler {
}
const aspectDeleteIds: Id64String[] = [];
const sql = `SELECT ECInstanceId,Identifier,JsonProperties FROM ${ExternalSourceAspect.classFullName} aspect WHERE aspect.Scope.Id=:scopeId AND aspect.Kind=:kind`;
this.targetDb.withPreparedStatement(sql, (statement: ECSqlStatement): void => {
await this.targetDb.withPreparedStatement(sql, async (statement: ECSqlStatement) => {
statement.bindId("scopeId", this.targetScopeElementId);
statement.bindString("kind", ExternalSourceAspect.Kind.Relationship);
while (DbResult.BE_SQLITE_ROW === statement.step()) {
Expand All @@ -722,6 +722,7 @@ export class IModelTransformer extends IModelExportHandler {
}
aspectDeleteIds.push(statement.getValue(0).getId());
}
await new Promise(setImmediate); // workaround for: https://github.com/nodejs/node-addon-api/issues/1140
}
});
this.targetDb.elements.deleteAspect(aspectDeleteIds);
Expand Down

0 comments on commit 6d9dba7

Please sign in to comment.