iTwin / imodel-transformer

API for exporting an iModel's parts and also importing them into another iModel
MIT License
3 stars 2 forks source link

lazy getElement #8

Open MichaelBelousov opened 1 year ago

MichaelBelousov commented 1 year ago

Currently shouldExportElement takes an entire element object, which means we query data from the source that we may never use to insert into the target. It might be possible to wrap shouldExportElement's element argument in a proxy and not actually load the element (load only its id and classFullName) unless some other property is requested.

This is potentially less useful because I'm not sure how many transformations ignore a large amount of elements excluding processChanges which will only call shouldExportElement on changed elements. This one requires some kind of measurement or analysis of existing transforms to determine if it would be useful. It could do a lot less loading, but perhaps in cases we don't really have often.

MichaelBelousov commented 1 year ago

@ViliusRuskys Do you know how often transformation service code's implementation of shouldExportElement checks element properties besides just the element Id and what the class is?

ViliusRuskys commented 1 year ago

The most common case is that we only care about the class and id of the element for filtering transformations. e.g. image

I only found 2 cases with the 'CreateFork' and 'MergeFork' transformations where we check the jsonProperties of the element if the element is an instanceof FunctionalPartition, but the 'MergeFork' transformation always only runs the change processing workflow and the 'CreateFork' transformation should always run on a clone of an iModel and its purpose is to only record the provenance, so it might be possible to skip the element exporting altogether.

It is nice having the Element object though. We would have to handle polymorphism on our own if we only had the classFullName.

MichaelBelousov commented 9 months ago

you wouldn't have to handle polymorphism because the change I'm proposing would still allow to easily get the element. Just by default only query the id/class id