Optimize MP for CMSSW CPU resources

[bryates]

This PR addresses an issue where CMSSW wastes CPU cycles for emptyUnit and nearFull3Unit. They are not loaded in static classes, and should only be initialized once.

[tomalin]

CMS coding rules state that class names should start with a capital letter, whereas variable and function names should start with a lower case letter.

[bryates]

Sorry I didn't get to these sooner. I've changed the names. Let me know if I missed anything.

[tomalin]

CMS coding rules state that class names should start with a capital letter, whereas variable and function names should start with a lower case letter.

[bryates]

Fixed

[tomalin]

CMS coding rules state that class names should start with a capital letter, whereas variable and function names should start with a lower case letter.

[bryates]

Fixed

[tomalin]

CMS coding rules state that class names should start with a capital letter, whereas variable and function names should start with a lower case letter.

[bryates]

Fixed

[tomalin]

This looks good to me, aside from the minor style comments above. We've not been able to test the CPU performance, as recent changes to the HLS L1Trk repo broke the Future SW again, but logically it should fix the problem.

[aehart]

This looks good to me, aside from the minor style comments above. We've not been able to test the CPU performance, as recent changes to the HLS L1Trk repo broke the Future SW again, but logically it should fix the problem.

This seems like an improvement, but is there a reason for not simply declaring the existing LUTs as static, instead of introducing these wrapper classes? For example, this PR does what I'm suggesting for one of the existing LUTs (not sure why this is only done for one of them):

firmware-hls/TrackletAlgorithm/MatchEngineUnit_parameters.h

Line 52 in 4faf3ca

static ap_uint<(1 << (2 * kNBitsBuffer))> lut;

The only thing missing would be logic to prevent reinitializing the LUT after the first call.

As it stands, multiple objects of these new wrapper classes can be declared, each of which will initialize the exact same LUT. In fact, I guess this happens, since objects of these classes are declared in MatchEngineUnit, of which there are multiple objects declared in MatchProcessor. Declaring the existing LUTs as static would prevent this.

[bryates]

This looks good to me, aside from the minor style comments above. We've not been able to test the CPU performance, as recent changes to the HLS L1Trk repo broke the Future SW again, but logically it should fix the problem.

This seems like an improvement, but is there a reason for not simply declaring the existing LUTs as static, instead of introducing these wrapper classes? For example, this PR does what I'm suggesting for one of the existing LUTs (not sure why this is only done for one of them):

firmware-hls/TrackletAlgorithm/MatchEngineUnit_parameters.h

Line 52 in 4faf3ca

static ap_uint<(1 << (2 * kNBitsBuffer))> lut;

The only thing missing would be logic to prevent reinitializing the LUT after the first call.
As it stands, multiple objects of these new wrapper classes can be declared, each of which will initialize the exact same LUT. In fact, I guess this happens, since objects of these classes are declared in MatchEngineUnit, of which there are multiple objects declared in MatchProcessor. Declaring the existing LUTs as static would prevent this.

@tomalin suggested the wrapper class to prevent CMSSW from recomputing the LUTs for every event. Things were set to static before and that didn't seem to be enough.

[aehart]

This looks good to me, aside from the minor style comments above. We've not been able to test the CPU performance, as recent changes to the HLS L1Trk repo broke the Future SW again, but logically it should fix the problem.

This seems like an improvement, but is there a reason for not simply declaring the existing LUTs as static, instead of introducing these wrapper classes? For example, this PR does what I'm suggesting for one of the existing LUTs (not sure why this is only done for one of them):

firmware-hls/TrackletAlgorithm/MatchEngineUnit_parameters.h

Line 52 in 4faf3ca

static ap_uint<(1 << (2 * kNBitsBuffer))> lut;

The only thing missing would be logic to prevent reinitializing the LUT after the first call.
As it stands, multiple objects of these new wrapper classes can be declared, each of which will initialize the exact same LUT. In fact, I guess this happens, since objects of these classes are declared in MatchEngineUnit, of which there are multiple objects declared in MatchProcessor. Declaring the existing LUTs as static would prevent this.

@tomalin suggested the wrapper class to prevent CMSSW from recomputing the LUTs for every event. Things were set to static before and that didn't seem to be enough.

I see that the existing LUT functions have static return types. However, my understanding is that in C++, this is meaningless, more or less (kind of like having a const return type). You have to declare the actual object within the function as static in order for this keyword to have an effect, as you do in the line I referenced above.

[bryates]

Good point. I had tried so many variations that didn't compile. Let me double check if that works.