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

Performance hit with numerous class initializations (2.0) #575

Closed
baseTwo opened this issue Sep 25, 2024 · 2 comments · Fixed by #585
Closed

Performance hit with numerous class initializations (2.0) #575

baseTwo opened this issue Sep 25, 2024 · 2 comments · Fixed by #585
Assignees
Labels

Comments

@baseTwo
Copy link
Collaborator

baseTwo commented Sep 25, 2024

This is the same ticket as below, only for the 2.0 branch:

@baseTwo baseTwo added this to the 2024 Q3 (September WGM) milestone Sep 25, 2024
@baseTwo baseTwo added bug Something isn't working Runtime labels Sep 25, 2024
@baseTwo baseTwo self-assigned this Sep 25, 2024
@baseTwo baseTwo linked a pull request Sep 25, 2024 that will close this issue
@baseTwo baseTwo changed the title Typeconverter is called for every new context (2.0) Performance hit with numerous class initializations (2.0) Sep 25, 2024
@baseTwo baseTwo removed the bug Something isn't working label Oct 7, 2024
@baseTwo baseTwo linked a pull request Oct 7, 2024 that will close this issue
@ewoutkramer
Copy link
Member

To improve over the 1.0 implementation, we need to make sure that:

  • If the CQL has only functions in a library, then it should be singleton
  • If has defines in it, it should not be singleton.
  • We should preserve the lazy behaviour for non-singleton classes.

Put otherwise: we should keep the old situation, except for CQL libraries that only contain CQL functions.

@ewoutkramer
Copy link
Member

An idea that we discussed was to put the memoization in the CqlContext. This would require you to be smart with the key, since the names of the functions can be aliased etc. We initially used Lazy, since it was faster, and we would need to do perf testing to see whether moving the state to CqlContext is slower.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment