Update getSchema in SchemaJsonLocater to be truly async #6734
Conversation
RohitPtnkr1996
requested review from
calebmshafer,
ColinKerr,
wgoehrig,
christophermlawson and
StefanApfel-Bentley
as code owners
| public async getSchema<T extends Schema>(schemaKey: Readonly<SchemaKey>, matchType: SchemaMatchType, context: SchemaContext): Promise<T | undefined> { | ||
| return this.getSchemaSync(schemaKey, matchType, context) as T; | ||
| public async getSchema<T extends Schema>(schemaKey: Readonly<SchemaKey>, _matchType: SchemaMatchType, context: SchemaContext): Promise<T | undefined> { | ||
| const schemaProps = this._getSchema(schemaKey.name); |
There was a problem hiding this comment.
so long as the SchemaPropsGetter is sync we will still be blocking. In @grigasp case the call to get schemas is a network request so we'd block until the request is completed.
There was a problem hiding this comment.
Not really...
Our library may be used both on the backend and the frontend, and my specific situation is about the backend. There, the IModelDb.getSchemaProps is sync and it seems there's not much you can do about that. The problem for us is that the whole SchemaJsonLocater.getSchema is synchronous and doesn't give a chance for other tasks to be executed until it's complete. Parsing the schema asynchronously should at least partially fix the problem (the first part - IModelDb.getSchemaProps may still take a long time, but at least the second part would be async). However, I'd like to point out that Schema.fromJSON lies about being async - it's actually calling fromJSONSync internally. This is why I had to add await BeDuration.wait(0) between getting schema props and parsing the schema in my custom locater that attempts to workaround the issue.
If schema props are retrieved using a network request, the function that does that MUST be async, which means it's incompatible with SchemaJsonLocater as it is defined now. On the frontend we use our library with ECSchemaRpcLocater.
There was a problem hiding this comment.
Good point.
We should have all the code necessary to write a property async schema from json ... The SchemaReadHelper has an async readSchema method that should behave the same a the readSchemaSync method. We should refactor that as part of this change.
wrt getting the json from the backend ... we should look into being able to do that off the main thread ... but that should be a separate issue.
There was a problem hiding this comment.
I'm sorry I think I misread this. Schema.fromJSON doesn't do any async operations so there is really no difference between a sync and an async version. I think the problem with this locater is that it returns the whole schema when you call 'getSchemaInfo'. The SchemaInfo is the schema key plus the referenced schemas keys. If we have this we can then make the request to load each reference asynchronously.
The ECSchemaRpcLocater does this correctly, I think we need to update this locater to match that behavior.
I don't think there is actually any need to change the fromJSON to have its own implementation at that point.
RohitPtnkr1996
changed the title
SchemaJsonLocater loads requested schema synchronously, which causes the main thread to block when loaded schema is large or has many referenced schemas.
SchemaJsonLocater.getSchema should have an asynchronous implementation instead of relying on a synchronous one.