Skip to content
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

Replace setImmediate usage with yield manager #3261

Merged
merged 7 commits into from
Feb 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions common/api/core-bentley.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1527,6 +1527,23 @@ export function utf8ToString(utf8: Uint8Array): string | undefined;
// @internal
export function utf8ToStringPolyfill(utf8: Uint8Array): string | undefined;

// @internal
export class YieldManager {
constructor(options?: YieldManagerOptions);
// (undocumented)
protected actualYield(): Promise<void>;
// (undocumented)
allowYield(): Promise<void>;
// (undocumented)
options: Readonly<Required<YieldManagerOptions>>;
}

// @internal
export interface YieldManagerOptions {
// (undocumented)
iterationsBeforeYield?: number;
}


// (No @packageDocumentation comment for this package)

Expand Down
5 changes: 4 additions & 1 deletion common/api/summary/core-bentley.exports.csv
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ public;ProcessDetector
public;PromiseReturnType
public;ReadonlyOrderedSet
public;ReadonlySortedArray
alpha;RealityDataStatus
beta;RepositoryStatus
beta;RpcInterfaceStatus
public;shallowClone
Expand All @@ -114,4 +115,6 @@ public;TransientIdSequence
public;UnexpectedErrors
public;using
public;utf8ToString(utf8: Uint8Array): string | undefined
internal;utf8ToStringPolyfill(utf8: Uint8Array): string | undefined
internal;utf8ToStringPolyfill(utf8: Uint8Array): string | undefined
internal;YieldManager
internal;YieldManagerOptions
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/core-bentley",
"comment": "add internal YieldManager",
"type": "none"
}
],
"packageName": "@itwin/core-bentley"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/core-transformer",
"comment": "",
"type": "none"
}
],
"packageName": "@itwin/core-transformer"
}
41 changes: 41 additions & 0 deletions core/bentley/src/YieldManager.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Bentley Systems, Incorporated. All rights reserved.
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/

/** @internal options for constructing yield managers */
export interface YieldManagerOptions {
iterationsBeforeYield?: number;
}

/** @internal the default options when constructing yield managers */
const defaultYieldManagerOptions: Required<YieldManagerOptions> = {
iterationsBeforeYield: 1000,
};

/**
* @internal
* An object allowing code to optionally yield with some frequency.
* useful in some intense loops that make processes unresponsive.
* primarily a workaround for: https://github.com/nodejs/node-addon-api/issues/1140
* @note see [[defaultYieldManagerOptions]], the default amount of times it must be called to cause an actual yield is 1000
*/
export class YieldManager {
public options: Readonly<Required<YieldManagerOptions>>;
private _counter = 0;

public constructor(options: YieldManagerOptions = {}) {
this.options = { ...defaultYieldManagerOptions, ...options };
}

public async allowYield() {
this._counter = (this._counter + 1) % this.options.iterationsBeforeYield;
if (this._counter === 0) {
await this.actualYield();
}
}

protected async actualYield() {
await new Promise((r) => setTimeout(r, 0));
}
}
1 change: 1 addition & 0 deletions core/bentley/src/core-bentley.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export * from "./StringUtils";
export * from "./Time";
export * from "./UnexpectedErrors";
export * from "./UtilityTypes";
export * from "./YieldManager";

/** @docs-package-description
* The core-bentley package contains classes to solve problems that are common for both client and server use cases.
Expand Down
22 changes: 22 additions & 0 deletions core/bentley/src/test/YieldManager.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Bentley Systems, Incorporated. All rights reserved.
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
import { expect } from "chai";
import { YieldManager } from "../YieldManager";

describe("YieldManager", () => {
it("allowYield yields once per 'iterationsBeforeYield' iterations", async () => {
class CountingYieldManager extends YieldManager {
public actualYieldCount = 0;
protected override async actualYield() { this.actualYieldCount++; }
}

const expectedYieldTimes = 5;
const yieldManager = new CountingYieldManager();
for (let i = 0; i < expectedYieldTimes * yieldManager.options.iterationsBeforeYield; ++i) {
await yieldManager.allowYield();
}
expect(yieldManager.actualYieldCount).to.equal(expectedYieldTimes);
});
});
8 changes: 5 additions & 3 deletions core/transformer/src/IModelExporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* @module iModels
*/

import { AccessToken, assert, DbResult, Id64, Id64String, IModelStatus, Logger } from "@itwin/core-bentley";
import { AccessToken, assert, DbResult, Id64, Id64String, IModelStatus, Logger, YieldManager } from "@itwin/core-bentley";
import { ECVersion, Schema, SchemaKey } from "@itwin/ecschema-metadata";
import { CodeSpec, FontProps, IModel, IModelError } from "@itwin/core-common";
import { TransformerLoggerCategory } from "./TransformerLoggerCategory";
Expand Down Expand Up @@ -440,6 +440,8 @@ export class IModelExporter {
return this.trackProgress();
}

private _yieldManager = new YieldManager();

/** Export the model contents.
* @param modelId The only required parameter
* @param elementClassFullName Can be optionally specified if the goal is to export a subset of the model contents
Expand Down Expand Up @@ -475,7 +477,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
await this._yieldManager.allowYield();
}
});
}
Expand Down Expand Up @@ -639,7 +641,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
await this._yieldManager.allowYield();
}
});
}
Expand Down
6 changes: 4 additions & 2 deletions core/transformer/src/IModelTransformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/
import * as path from "path";
import * as Semver from "semver";
import { AccessToken, DbResult, Guid, Id64, Id64Set, Id64String, IModelStatus, Logger, LogLevel, MarkRequired } from "@itwin/core-bentley";
import { AccessToken, DbResult, Guid, Id64, Id64Set, Id64String, IModelStatus, Logger, LogLevel, MarkRequired, YieldManager } from "@itwin/core-bentley";
import * as ECSchemaMetaData from "@itwin/ecschema-metadata";
import { Point3d, Transform } from "@itwin/core-geometry";
import {
Expand Down Expand Up @@ -698,6 +698,8 @@ export class IModelTransformer extends IModelExportHandler {
});
}

private _yieldManager = new YieldManager();

/** Detect Relationship deletes using ExternalSourceAspects in the target iModel and a *brute force* comparison against relationships in the source iModel.
* @see processChanges
* @note This method is called from [[processAll]] and is not needed by [[processChanges]], so it only needs to be called directly when processing a subset of an iModel.
Expand All @@ -722,7 +724,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
await this._yieldManager.allowYield();
}
});
this.targetDb.elements.deleteAspect(aspectDeleteIds);
Expand Down