BHoM / BHoM_Engine

Internal manipulation of the BHoM
GNU Lesser General Public License v3.0
26 stars 15 forks source link

Versioning_Engine: Add methodology for directly calling upgrades from main process rather than via pipes #3332

Closed IsakNaslundBh closed 7 months ago

IsakNaslundBh commented 7 months ago

NOTE: Depends on

https://github.com/BHoM/Versioning_Toolkit/pull/257

Issues addressed by this PR

Closes #3331

Changes the way the BHoMUpgraders are called from running via pipes to instead dynamically loading up the upgraders and directly calling them.

Initial testing shows that this can give a more than 10x speed increase for de-serialising objects requiring versioning, which can be critical for slightly larger serialised files. Increasingly important after changes like the one made in https://github.com/BHoM/BHoM/pull/1579 as that would significantly slow down the de-serialisation of and json file containing Bar objects. This could mean a difference for a file containing a few 100 bars of 10-30 seconds rather than say 3-5 minutes. Hence think this change is fairly critical.

For this PR I opted to make the events raised and calls make mimic the current system as 1:1 as possible. Scope to clean up some of the event raising and similar, but would prefer to leave that to a second PR as this change is big enough.

Also opted to keep the code for the Pipe system, and having a hard-coded boolean as to which system to be used, to make it very simple to switch back to the old system if a problem is discovered without completely reverting this PR. Long term if this works, I think we can get rid of it, and discussed with @adecler , scope to also significantly simplifying the versioning system, leading to a single dll rather than the 1 exe per version we currently work with, but again, for a later PR.

Test files

Tested through as complete as I could think of here

Test by de-serialising all files in https://github.com/BHoM/Versioning_Toolkit/tree/develop/.ci/code/Versioning_Test/Datasets and then serialise them out again, first using the current system on develop (folder called Old in the test folder) and then doing the same again with the new system on this branch. Whilst doing the runs, all events for a particular object is also recorded and stored out.

Every re-serialised json is then compared, ensuring the content contained is the same. Same is done for the events, ensuring the events raised are identical.

For Methods and adapters, no change found.

For obejcts, the only difference found has been properties with BHoM_Guid for some object types. From investigation it seem to be Guids of things like CustomObjects and a like, where a new Guid has been instantiated during the De-serialisation, hence natural that a difference occurs (would be the same as comparing two runs of the old system with each other).

The only other difference found has to do with Items containing "$date" and relating to objects containing Events from 3.3 and before. For this it is related to this commit: https://github.com/BHoM/BHoM/commit/d5998b2c6681dcb6362a614b21ce1648af1970a9 where the date time was added. And object serialised before that will naturally deserialise to include a new DateTime = DateTime.UtcNow which then simply depends on when it was deserialised.

On top of this, ofc Versioning check should be passing. Have also added an example json file and script that can be used to check the speed difference.

Changelog

Additional comments

IsakNaslundBh commented 7 months ago

@BHoMBot check versioning

bhombot-ci[bot] commented 7 months ago
@IsakNaslundBh to confirm, the following actions are now queued: - check `versioning`
IsakNaslundBh commented 7 months ago

@BHoMBot check compliance

bhombot-ci[bot] commented 7 months ago
@IsakNaslundBh to confirm, the following actions are now queued: - check `code-compliance` - check `documentation-compliance` - check `project-compliance` - check `branch-compliance` - check `dataset-compliance` - check `copyright-compliance`
IsakNaslundBh commented 7 months ago

@BHoMBot check compliance

bhombot-ci[bot] commented 7 months ago
@IsakNaslundBh to confirm, the following actions are now queued: - check `code-compliance` - check `documentation-compliance` - check `project-compliance` - check `branch-compliance` - check `dataset-compliance` - check `copyright-compliance`
IsakNaslundBh commented 7 months ago

@BHoMBot check versioning

bhombot-ci[bot] commented 7 months ago
@IsakNaslundBh to confirm, the following actions are now queued: - check `versioning`
IsakNaslundBh commented 7 months ago

@BHoMBot check installer @BHoMBot check core @BHoMBot check serialisation @BHoMBot check null-handling

bhombot-ci[bot] commented 7 months ago
@IsakNaslundBh to confirm, the following actions are now queued: - check `installer` - check `core` - check `serialisation` - check `null-handling` There are 2 requests in the queue ahead of you.
IsakNaslundBh commented 7 months ago

@BHoMBot check unit-tests

bhombot-ci[bot] commented 7 months ago
@IsakNaslundBh to confirm, the following actions are now queued: - check `unit-tests` There are 24 requests in the queue ahead of you.
IsakNaslundBh commented 7 months ago

@BHoMBot check compliance @BHoMBot check required

bhombot-ci[bot] commented 7 months ago
@IsakNaslundBh to confirm, the following actions are now queued: - check `code-compliance` - check `documentation-compliance` - check `project-compliance` - check `branch-compliance` - check `dataset-compliance` - check `copyright-compliance` - check `code-compliance` - check `documentation-compliance` - check `project-compliance` - check `core` - check `null-handling` - check `serialisation` - check `versioning` - check `installer` There are 46 requests in the queue ahead of you.
bhombot-ci[bot] commented 7 months ago
The check `code-compliance` has already been run previously and recorded as a successful check. This check has not been run again at this time.
bhombot-ci[bot] commented 7 months ago
The check `documentation-compliance` has already been run previously and recorded as a successful check. This check has not been run again at this time.
IsakNaslundBh commented 7 months ago

@BHoMBot check unit-tests

bhombot-ci[bot] commented 7 months ago
@IsakNaslundBh to confirm, the following actions are now queued: - check `unit-tests` There are 3 requests in the queue ahead of you.
bhombot-ci[bot] commented 7 months ago
FAO: @FraserGreenroyd @IsakNaslundBh is seeking dispensation on this Pull Request to skip a required check. Please can you provide authorisation for the check to be skipped, or provide assistance as appropriate. The check they wish to have dispensation on is unit-tests. If you are providing dispensation on this occasion, please reply with: > @BHoMBot this is a DevOps instruction. I am authorising dispensation to be granted on check ref. `24148984891`
IsakNaslundBh commented 7 months ago

@BHoMBot this is a DevOps instruction. I am authorising dispensation to be granted on check ref. 24148984891

bhombot-ci[bot] commented 7 months ago
@IsakNaslundBh I have now provided a passing check on reference `24148984891` as requested.
IsakNaslundBh commented 7 months ago

@BHoMBot check ready-to-merge

bhombot-ci[bot] commented 7 months ago
@IsakNaslundBh to confirm, the following actions are now queued: - check `ready-to-merge` There are 2 requests in the queue ahead of you.