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

ExtractHexDigits high CPU Usage + Time #3133

Closed
bastigerner opened this issue Nov 18, 2021 · 7 comments
Closed

ExtractHexDigits high CPU Usage + Time #3133

bastigerner opened this issue Nov 18, 2021 · 7 comments
Assignees
Labels
Type: Bug 🐞 Something isn't working

Comments

@bastigerner
Copy link

bastigerner commented Nov 18, 2021

Using Theme Provider slows down the whole application. This is caused by ExtractHexDigits being called multiple 1000 times.
Taking up to 3500ms for 2500 calls.

Use the method:

protected static string ExtractHexDigits( string input )

Fill it with any hex color string and run it 2500 times in a loop.
This will take arround 2600ms.

2500 runs taking only couple of ms.

Simple fix to reduce the time and usage drasticly:

private static Regex isHexDigit = new( "[abcdefABCDEF\\d]+", RegexOptions.Compiled );

    protected static string ExtractHexDigits( string input )
    {
        // remove any characters that are not digits (like #)
        string newnum = "";
        foreach ( char c in input )
        {
            if ( isHexDigit.IsMatch( c.ToString() ) )
                newnum += c.ToString();
        }
        return newnum;
    }

Declaring the same regex each time the function is called slows it down.

@stsrki stsrki added this to the 0.9.5 Support milestone Nov 18, 2021
@stsrki stsrki added the Type: Bug 🐞 Something isn't working label Nov 18, 2021
@bastigerner
Copy link
Author

This bug gets extremely noticable when using a grid with a huge amount of data and switching between pages with different grids.

Shortly tested again. Without static declaration of the regex 50 calls take 91ms. Declaring the regex outside of the method and calling it 10.000 times only takes 42ms.

@David-Moreira
Copy link
Contributor

Nice catch, thanks for troubleshooting.
Regex should generally be cached in hot paths, yes. We'll benchmark and take a look! Thanks!

@bastigerner
Copy link
Author

Sure no problem!
Also I was a bit curious what the real profit of this function is?
Basically it clears the string to be a hex string but why is it needed?

@stsrki
Copy link
Collaborator

stsrki commented Nov 19, 2021

It is used just as an extra step to make sure the end string contains a valid value that can be converted to the real Color.

@bastigerner
Copy link
Author

Hm okay we have also thought that. So it is kind of much needed? Otherwise I would have just removed it 🤣

@stsrki
Copy link
Collaborator

stsrki commented Nov 19, 2021

I would leave. You have no idea what users could try to enter :)

@bastigerner
Copy link
Author

Fair enough haha 🤣

@stsrki stsrki closed this as completed Nov 20, 2021
@stsrki stsrki added this to Support Aug 3, 2024
@stsrki stsrki moved this to ✔ Done in Support Aug 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug 🐞 Something isn't working
Projects
Archived in project
Development

No branches or pull requests

3 participants