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

Avoid string StartsWith and EndsWith #2142

Merged
merged 1 commit into from
Nov 27, 2017

Conversation

tsholmes
Copy link
Member

String functions in mono are extremely slow. This avoids calling StartsWith and EndsWith for cases where we could just check the characters directly.

This had a varying effect from almost no speedup to an 8% speedup in my test cases depending on how call heavy the script was (the EndsWith("()") check is the most expensive).

@tsholmes tsholmes force-pushed the fix/remove_string_with branch 2 times, most recently from 85709fd to 55f5235 Compare October 27, 2017 01:07
Copy link
Member

@Dunbaratu Dunbaratu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know what you think, @tsholmes. I am ready to merge this, but was wondering about this one point.

return str.Length > 0 && str[0] == '$';
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we just have a generic "endswith" and "startswith" call in our utility library that operates on generic strings but does it char by char? Would it be an improvement over the generic mono one that way? i.e:

public static bool EndsWith(string biggerString, string substring)
{
    int len = subString.Length;
    int i = biggerString.Length - len;
    if (i<0) return false;
    while (i < len)
    {
        if (subSrring[i] != biggerString[j]) // maybe allow for culture case-insensitive comparison here?  Nah, probably not.
            return false;
        ++i;
        ++j;
    }
    return true;
}

It seems weird to be writing one hardcoded primitive method per endswith comparison we want to do, but I can conceed it may be worth it if it's faster that the generic solution.

@tsholmes tsholmes force-pushed the fix/remove_string_with branch 2 times, most recently from 1e5338f to 9a3abf4 Compare November 26, 2017 21:52

for (int i = 0; i < suffixLen; i++)
{
if (str[strLen - i - 1] != suffix[suffixLen - i - 1])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to switch to counting this forward to save some math. (set index to strLen - i - 1 at first, then just increment it by +1 each iteration).

@Dunbaratu Dunbaratu merged commit 9a3abf4 into KSP-KOS:develop Nov 27, 2017
@Dunbaratu Dunbaratu added this to the v1.1.4.0 milestone Dec 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants