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

IsMultiplexed run time #37

Open
taylorjdlee opened this issue Jul 17, 2023 · 3 comments
Open

IsMultiplexed run time #37

taylorjdlee opened this issue Jul 17, 2023 · 3 comments
Labels
enhancement New feature or request
Milestone

Comments

@taylorjdlee
Copy link
Contributor

taylorjdlee commented Jul 17, 2023

public static bool IsMultiplexed(this Message message)

Due to using the any() method IsMultiplexed() can take sometime to run if there are a lot of signals present in the message. It would be great if we could have a simple boolean in the message that means we don't have to iterate across every signal to improve the runtime of this function.

But I also think it's down to the runtime of MultiplexingInfo

public static MultiplexingInfo MultiplexingInfo(this Signal signal)

Pretty much I want IsMultiplexed() to be efficient as possible due to using this library to decode data at runtime. Maybe the message class can even have a boolean value stored that does this? Pretty much when we initialise a message when loading from the .dbc we run the IsMultiplexed() function once to generate this boolean value

@Adhara3
Copy link
Collaborator

Adhara3 commented Nov 6, 2023

Hi @taylorjdlee ,

To be honest, this

Pretty much I want IsMultiplexed() to be efficient as possible due to using this library to decode data at runtime.

sounds weird.
I do develop runtime systems and I want to share my experience on this. All static (i.e. that do not change during the "acquisition") informations should be used to prepare the system so that the minimum possible number of operations should be executed at runtime. So, if signal is multiplexed and will require further processing then you should set up your processing system the right way not ask you at every packet if that packet requires muxing processing or not. Put in another way, use the library to instantiate the right classes instead of having a single class that works for every signal and should then adapt at runtime to every possible combination.

Beside the performance reason, there is a conceptual reason to me: if you keep asking is signal multiplexed? (in code terms if(signal.IsMultiplexed()) then this implies that the answer may change which, for a given signal, is not true until you change your dbc.

So, ok, I will have a look at performances but please consider instantiate classes that know what to do upfront for best performance results.

Regards
A

@Adhara3 Adhara3 added the enhancement New feature or request label Nov 6, 2023
@Uight
Copy link
Contributor

Uight commented Jul 26, 2024

I pretty much agree with @Adhara3 on this but i guess it could be helpfull to have a IsMultiplexed Property instead of the ExtensionMethod. I wouldnt be using a method for something that cant change.
Its pretty much just a nice thing as then you wouldnt have to store it in your own "wrapping" class for message.

Otherwise theres not much to optimize the code of IsMultiplexed(). However if you are using Sonarqube it suggests using Exists instead of Any. (https://sonarsource.github.io/rspec/#/rspec/S6605/csharp)
But i doubt that would make a difference as the Signal list is always pretty small. Havents seen any can message with more than 30 signals.

Uight added a commit to Uight/DbcParser that referenced this issue Jul 29, 2024
…r for code cleanup; Add IsMultiplexed to signal
Uight added a commit to Uight/DbcParser that referenced this issue Aug 20, 2024
…eadonly for public access. Use FinishUp method to calc useful properties.
@Adhara3 Adhara3 added this to the 2.0 milestone Aug 31, 2024
@Adhara3
Copy link
Collaborator

Adhara3 commented Aug 31, 2024

Will be part of the immutability refactoring and will I Claude the extended multiplexing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
3 participants