547 Static classes and libraries on 1.0

[DamienMosier]

• update the code generator to turn all of files into static and remove the required initialization

Observed performance gains

• 4.8 minutes down to 2 minutes
• 38 minutes down to 11.7 minutes

Performance gains of almost 60% by removing the Lazy loading/caching and moving to just static classes and passing the context around.

Fix for #547

[baseTwo]

What does the code generated files look like now? Could you include this?

[DamienMosier]

Here is an snippet from the updated FHIRHelpers class

[baseTwo]

And what is the performance if only the lazy initialization is removed while keeping classes non-static? I'd like to see an example of the performance test itself too.

[DamienMosier]

Per the screenshot in the initial bug, the time sink was due to the numerous class initializations and calling all of the constructors. Removing all of that reduces the impact. The Lazy was used for caching and not recalculating the value of a define.

[baseTwo]

I have included the changes in 2.0 just to look at the changes in the generated libraries. Will have to discuss with @ewoutkramer if this is the right approach

• PR 575 Investigate static libraries on 2.0 #549

[baseTwo]

Just some thoughts

• I have no way of seeing how the libraries are used in your test, or to see the before/after metrics on them
• I'm fine with removing the Lazy's
• Making everything static is not the ideal way to solve this. I'd rather that the libraries get re-used between each test case.
• For 2.0 our solution would inject library dependencies in each library type constructor, and have the service container resolve them and manage their lifetime.
• If you want to keep the 1.0 as it is, just be prepared there will be some work to convert them to 2.0. If I have time, I'll update the 2.0 implementation in this PR 575 Investigate static libraries on 2.0 #549

I discussed with @ewoutkramer and he'll reach out to @EvanMachusak on this too

[DamienMosier]

so if you look at AAB from the DQIC project you will notice that when you initialize the AAB_Reporting_2023_0_1 class it will initialize the libraries being used and the libraries used inside of those, etc. This chews up time having to initialize everything. In the 1.0 branch, which we are currently using, swapping to static improves runtime drastically thus improving time and saving memory. A 60% performance boost is a massive savings in time and money over the current process for the 1.0 branch being used in our software.

[System.CodeDom.Compiler.GeneratedCode(".NET Code Generation", "1.0.0.0")]
[CqlLibrary("AAB_Reporting", "2023.0.1")]
public class AAB_Reporting_2023_0_1
{
    internal CqlContext context;

    #region Cached values

    internal Lazy<Tuples.Tuple_BOFQdLfCiMegGCeQZaSIJRaC> __Measure;
    internal Lazy<Patient> __Patient;
    internal Lazy<IEnumerable<ExplanationOfBenefit>> __Eobs;
    internal Lazy<IEnumerable<Claim>> __Claims;
    internal Lazy<IEnumerable<CqlValueSet>> __Competing_Diagnoses;
    internal Lazy<CqlInterval<CqlDate>> __Measurement_period;
    internal Lazy<CqlInterval<CqlDate>> __Intake_period;
    internal Lazy<CqlInterval<int?>> __Initial_population_age_range_in_months;
    internal Lazy<CqlQuantity> __minAgeLimit;
    internal Lazy<IEnumerable<CqlDate>> __IP_eligible_episodes_without_CE_check;
    internal Lazy<IEnumerable<CqlDate>> __Eligible_event_dates;
    internal Lazy<bool?> __Initial_population_must_be_continuously_enrolled;
    internal Lazy<IEnumerable<Coverage>> __Coverages;
    internal Lazy<IEnumerable<CqlDate>> __Initial_population;
    internal Lazy<bool?> __Exclude_deceased_members;
    internal Lazy<bool?> __Exclude_members_with_hospice;
    internal Lazy<IEnumerable<Encounter>> __Encounters;
    internal Lazy<IEnumerable<Procedure>> __Procedures;
    internal Lazy<bool?> __Exclusion;
    internal Lazy<IEnumerable<CqlDate>> __Denominator;
    internal Lazy<IEnumerable<ClaimResponse>> __ClaimResponses;
    internal Lazy<bool?> __Has_paid_antibiotic_dispenses;
    internal Lazy<IEnumerable<CqlDate>> __Numerator;
    internal Lazy<CqlInterval<int?>> __Stratification_1_age_range_in_months;
    internal Lazy<IEnumerable<CqlDate>> __Age_Stratification_1;
    internal Lazy<CqlInterval<int?>> __Stratification_2_age_range_in_years;
    internal Lazy<IEnumerable<CqlDate>> __Age_Stratification_2;
    internal Lazy<CqlInterval<int?>> __Stratification_3_age_range_in_years;
    internal Lazy<IEnumerable<CqlDate>> __Age_Stratification_3;
    internal Lazy<IEnumerable<CqlDate>> __IP_eligible_episodes_with_CE_check;
    internal Lazy<CqlDate> __AABA_event_date;
    internal Lazy<IEnumerable<string>> __AABA_Payers;
    internal Lazy<int?> __Count_of_IP_eligible_episodes_without_CE_check;
    internal Lazy<CqlDate> __AABB_event_date;
    internal Lazy<IEnumerable<string>> __AABB_Payers;
    internal Lazy<bool?> __AABA_CE;
    internal Lazy<bool?> __AABB_CE;
    internal Lazy<bool?> __AABA_Event;
    internal Lazy<bool?> __AABB_Event;
    internal Lazy<IEnumerable<CqlDate>> __Eligible_episodes;
    internal Lazy<bool?> __AABA_Epop;
    internal Lazy<bool?> __AABB_Epop;
    internal Lazy<bool?> __AABA_Num;
    internal Lazy<bool?> __AABB_Num;
    internal Lazy<bool?> __RExcl;
    internal Lazy<bool?> __RExclD;
    internal Lazy<int?> __AABA_Age;
    internal Lazy<int?> __AABB_Age;
    internal Lazy<string> __Gender;
    internal Lazy<CqlDate> __First_event_with_CE;
    internal Lazy<CqlDate> __Last_event_with_CE;

    #endregion
    public AAB_Reporting_2023_0_1(CqlContext context)
    {
        this.context = context ?? throw new ArgumentNullException("context");

        HEDIS_2023_0_0 = new HEDIS_2023_0_0(context);
        Certification_Payer_Reporting_2023_0_0 = new Certification_Payer_Reporting_2023_0_0(context);
        Hospice_2023_0_0 = new Hospice_2023_0_0(context);
        Antibiotics_Shared_Elements_2023_0_0 = new Antibiotics_Shared_Elements_2023_0_0(context);
        Antibiotics_Shared_Concepts_2023_0_0 = new Antibiotics_Shared_Concepts_2023_0_0(context);
        AAB_Concepts_2023_0_0 = new AAB_Concepts_2023_0_0(context);
        FHIRHelpers_4_0_1 = new FHIRHelpers_4_0_1(context);
        Claims_2023_0_0 = new Claims_2023_0_0(context);
        ExplanationOfBenefits_2023_0_0 = new ExplanationOfBenefits_2023_0_0(context);
        Patients_2023_0_0 = new Patients_2023_0_0(context);
        Antibiotic_2023_0_0 = new Antibiotic_2023_0_0(context);
        FHIR_Common_2023_0_0 = new FHIR_Common_2023_0_0(context);

[DamienMosier]

Problem is when you have to initialize the context for each patient because the bundle exists in the context and you cannot just swap out the bundle to reuse everything else. If you could then you would have to remove the Lazy or reset them. Thus initializing each class causes a drastic hit to performance. I have not looked at the 2.0 branch as I am doing performance tuning for our measures and uncovering that the engine is the worst offender.

[baseTwo]

I have updated the 2.0 implementation

• 575 Investigate static libraries on 2.0 #549

This still has to be discussed with @ewoutkramer and @EvanMachusak

• The library types are non-static.
• They accept dependencies to other libraries via a primary constructor
• Each method accept the CqlContext, just like in the new 1.0 implementation
• Each library type also has an extension type on IServiceCollection , to register the library and its dependencies

The DQIC branch which uses the service provider to create libraries is available here.

[DamienMosier]

updated with the requested changes

[EvanMachusak]

There is some performance benefit by using static invocations over instance ones, but it is not clear what that is. The trick with CQL in particular is that it is run on large data sets - in our case many 10m+ patients - and even small performance changes can save many minutes on large runs.

I think the DI solution should make approximately the same gains as everything being pure static and is more harmonious with the rest of the 2.0 SDK.

It would be interesting to run a test where everything is equal except the 1.0 vs. 2.0 SDK to see which performs better on the same measure and same patient set. I would hope we don't go backward in performance.

[baseTwo]

Could you test the performance on singleton libraries? My expectation is that it should be comparable to static libraries.

#552

[baseTwo]

Small change on the method name. If we're happy that singleton types can work comparable to performance to static ones, then this change is already in the commit for cherry picking 3116f41 (PR #549)

[baseTwo]

Rename to WriteMemoizedInstanceMethod to WriteMethod. See review comment above first

[DamienMosier]

Tested the singleton approach and it produced similar results so either works

[DamienMosier]

closing this using static to use the singleton PR

[baseTwo]

closing this using static to use the singleton PR

Out of curiosity, was there any difference in performance?

[DamienMosier]

I had previously mentioned here and in your PR that it was comparable and said to just use your PR because it had all the test updates but nothing has been merged so I am in the process of pulling your changes in for the tests so we can get this finally merged in to see those massive performance gains.

[DamienMosier]

@ewoutkramer this is the PR we spoke about this past weekend. Can we get this reviewed and merged into the 1.0 branch?

[baseTwo]

Out of curiosity again, what are the performance gains on .NET 9? Microsoft has made huuuuge improvements in performance and much less memory allocation on LINQ