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

Reflection_Engine: deserialization incl. versioning is extremely slow, wouldn't it be worth informing the user this will take time? #2411

Closed
pawelbaran opened this issue Mar 22, 2021 · 4 comments
Assignees
Labels
type:question Ask for further details or start conversation

Comments

@pawelbaran
Copy link
Member

Description:

As in the title, deserialising a single BHoM object with versioning being involved takes up to 500ms (at least on my examples). Multiplied by a number of elements in a project model, this results in software freezing for long minutes on startup - I am pretty sure most of the users will kill the process before it finishes, convinced that it simply crashed.

Assuming that we cannot do much about speeding up deserialisation without massive effort, a simple solution for the issue above could be a popup to inform the user about the possible slowdown (and ways to avoid it in the future) once the need for versioning is identified. What do you think?

@pawelbaran pawelbaran added the type:question Ask for further details or start conversation label Mar 22, 2021
@adecler
Copy link
Member

adecler commented Mar 23, 2021

Thanks @pawelbaran , good point. Two initial comments on that:

  • Versioning is slower than before because we switched to forward versioning without exposing the version on the serialised objects. This means that every object that needs an upgrade goes through the entire chain of upgrader. Once we add the version number into the serialisation (beginning of next quarter), speed will be a lot less of an issue. It should even be faster than backward versioning.

  • We don't have any mechanism right now to generate warnings on the component in the middle of a run. And, given how deep versioning is, getting that feature will require some serious thoughts. We could obviously do an actual popup window generated directly by the versioning engine but I am not sure I love this idea. Given how annoying those popup windows can be, a minimum we would need to

    • make sure we pop that window only once,
    • only if the objects to upgrade lack a version number
    • estimate computational time (number of objects would do) and only show popup if above a given threshold

@pawelbaran
Copy link
Member Author

Great to hear that we have a performance improvement in the pipeline @adecler! Regarding the popup, I was thinking about showing one on startup of the script, i.e. when the user opens the file, it gets checked whether it was last saved on the same BHoM version - if not, show a generic window with a brief message saying that 'you are on 4.X, while the file was saved in 3.X and contains serialised objects, therefore versioning will be needed which may take time' etc. Possibly it is too simplistic, but may be a worthy starting point to consider? Just an idea, not sure if still needed after the future changes you have mentioned.

@adecler
Copy link
Member

adecler commented Mar 25, 2021

when the user opens the file, it gets checked whether it was last saved on the same BHoM version - if not, show a generic window with a brief message saying that 'you are on 4.X, while the file was saved in 3.X and contains serialised objects, therefore versioning will be needed which may take time' etc.

I would love to do that 😄 . For that, we need to have a way to recover the version the file was saved on. We can either recover that from the file template or from the version number saved with serialisation. Both of those features have only just been added though so we cannot recover BHoM version from files prior to this date. Since this is the lack of knowledge about the version to start versioning from that causes it to be slow, the times when the popup would be the most useful is also the times when we don't have access to the info to generate the message.

I still think the idea has value though. We know that anything prior to 4.2 doesn't have a version number saved on its serialisation so we know it will potentially take time to deserialise. We can then show a dialog box the first time it occurs. The only thing I am still a bit hesitant about is that the Versioning Engine will depend on a UI dll (WinForm).

@FraserGreenroyd
Copy link
Contributor

Following a review of this issue with @adecler during issue closure, I ran some tests of speed of the FromJson deserialisation for objects that need versioning and objects that don't.

For this test, I used all 1134 objects from 6.0 in the versioning test sets, and these deserialised in 13.6 seconds for all objects. Singling them out 1 by 1 deserialised fast enough for the Grasshopper profiler to not want to tell me how long it took for the component to run.

I then used all 900 objects from the 3.3 dataset, and these deserialised in 15.4 seconds for all objects, some of which will have needed to be upgraded via versioning. Singling them out 1 by 1 resulted in the largest wait of 121ms for a single object that needed versioning.

I'm of the opinion that this is reasonable for now so we can say this has been solved in the interim period since this issue was raised, but if anyone disagrees then we can reopen this issue and take further looks 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:question Ask for further details or start conversation
Projects
None yet
Development

No branches or pull requests

6 participants