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.

Sign up for GitHub

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

Jump to bottom

Add caching system for CRUD methods #337

Merged
merged 15 commits into from
Jan 23, 2023
Merged

Add caching system for CRUD methods #337

merged 15 commits into from
Jan 23, 2023

Conversation

IsakNaslundBh
Copy link
Contributor

Issues addressed by this PR

Closes #336

Add caching system in the base adapter, reducing the times the same objects are read more than once from the model.

Can for some cases significantly speed up both push and pulls from the adapter, depending o the case being run.

For full effect, the concrete adapters need to be updated to make use of this new system.

Test files

Changelog

Additional comments

@IsakNaslundBh IsakNaslundBh added the type:feature New capability or enhancement label Jan 17, 2023
@IsakNaslundBh IsakNaslundBh requested a review from alelom January 17, 2023 11:51
@IsakNaslundBh IsakNaslundBh self-assigned this Jan 17, 2023
This was referenced Jan 17, 2023
Closed
Merged
This was referenced Jan 18, 2023
Closed
Closed
@IsakNaslundBh IsakNaslundBh mentioned this pull request Jan 18, 2023
Merged
@alelom alelom changed the title Add caching system for crud methods Add caching system for CRUD methods Jan 18, 2023
IsakNaslundBh added a commit to BHoM/GSA_Toolkit that referenced this pull request Jan 19, 2023
@IsakNaslundBh
Update to make sue of caching system added in BHoM/BHoM_Adapter#337
3c4e694 
Also includes a tweak to read of materials to aim to make it fit with new system.
ID for materials for GSA10 now includes material type, as that is required to differentiate between different material types
@IsakNaslundBh IsakNaslundBh mentioned this pull request Jan 19, 2023
Merged
alelom
alelom reviewed Jan 19, 2023
View reviewed changes
BHoM_Adapter/CRUD/Cache.cs Outdated Show resolved Hide resolved
alelom
alelom reviewed Jan 19, 2023
View reviewed changes
BHoM_Adapter/CRUD/Cache.cs Outdated Show resolved Hide resolved
@IsakNaslundBh @alelom
Apply suggestions from code review
ddd2557 
Co-authored-by: Alessio Lombardi <[email protected]>
@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check compliance
@BHoMBot check core

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 20, 2023

@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 core

@IsakNaslundBh
Fix for test calling the correct cache method
4c40950
@IsakNaslundBh
Change default value for UpdateOnlyChangedObjects to true
e79df07 
Discussing with @alelom that this should be the default for most if not all cases, with opt out being the exception case where Toolkits have to set this parameter
@alelom alelom self-requested a review January 20, 2023 15:35
alelom
alelom approved these changes Jan 20, 2023
View reviewed changes
Copy link
Member

@alelom alelom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy with the functionality. Code now looks good to me. Tested on Robot/GSA toolkits via the related PRs and the create-read test script for structural adapters.

@alelom
Copy link
Member

alelom commented Jan 20, 2023

@BHoMBot check compliance
@BHoMBot check core

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 20, 2023

@alelom 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 core

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check required

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 23, 2023

@IsakNaslundBh to confirm, the following actions are now queued:

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check core
  • check null-handling
  • check serialisation
  • check versioning
  • check installer

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 23, 2023

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
Copy link

bhombot-ci bot commented Jan 23, 2023

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.

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 23, 2023

The check core has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 23, 2023

The check installer has already been run previously and recorded as a successful check. This check has not been run again at this time.

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 23, 2023

@IsakNaslundBh to confirm, the following actions are now queued:

  • check ready-to-merge

@IsakNaslundBh IsakNaslundBh merged commit 558475d into develop Jan 23, 2023
@IsakNaslundBh IsakNaslundBh deleted the BHoM_Adapter-#336-AddCachingSystemForCRUDMethods branch January 23, 2023 07:58
IsakNaslundBh added a commit to BHoM/GSA_Toolkit that referenced this pull request Jan 27, 2023
@IsakNaslundBh
Update to make sue of caching system added in BHoM/BHoM_Adapter#337
b8ca3c1 
Also includes a tweak to read of materials to aim to make it fit with new system.
ID for materials for GSA10 now includes material type, as that is required to differentiate between different material types
@bhombot-ci bhombot-ci bot mentioned this pull request Mar 13, 2023
Merged
FraserGreenroyd pushed a commit to BHoM/GSA_Toolkit that referenced this pull request Mar 28, 2023
@IsakNaslundBh
Update to make sue of caching system added in BHoM/BHoM_Adapter#337
9d710e8 
Also includes a tweak to read of materials to aim to make it fit with new system.
ID for materials for GSA10 now includes material type, as that is required to differentiate between different material types
This was referenced Jun 28, 2023
Closed
Merged
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alelom alelom alelom approved these changes

Assignees

@IsakNaslundBh IsakNaslundBh

Labels
type:feature New capability or enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

BHoM_Adapter: Add caching system for CRUD methods
2 participants
@IsakNaslundBh @alelom