Skip to content

The implementation of ToCamelCase can be optimized #230

Open

Description

@wmjordan

I am studying the source code today and found that ToCamelCase could be optimized.

The current implementation stack allocates a new char array, works on it and create a string from that char array.

You can use String.Copy to copy an instance of a string, and directly pin the string with a fixed statement and work on that copy. This saves one allocation and quite a few char assignments.

Activity

  1. rpgmaker commented on Aug 10, 2020

    @rpgmaker
    Owner

    Thanks for the idea. Would be nice to see a before and after numbers with an example .

  2. wmjordan commented on Aug 10, 2020

    @wmjordan
    ContributorAuthor

    The implementation of that function sounds a little bit weird to me.

    ToCamelCase("ABC_CD") => "aBCCD"
    ToCamelCase("ABC_cD") => "aBCCD"

    Why does it remove the underscore and uppercase the following character?

    EDIT:
    Oh, it seems that it is supposed to deal with the following situation:

    ToCamelCase("Abc_De") => "abcDe"
    ToCamelCase("Abc de") => "abcDe"

  3. rpgmaker commented on Aug 10, 2020

    @rpgmaker
    Owner

    Cool. Np

  4. rpgmaker commented on Apr 4, 2021

    @rpgmaker
    Owner

    Are intending to still create a pull request for this optimization? Thanks

  5. wmjordan commented on Apr 5, 2021

    @wmjordan
    ContributorAuthor

    It is too long time ago that I have forgotten this.

  6. wmjordan commented on Apr 5, 2021

    @wmjordan
    ContributorAuthor

    Here's my implementation of ToCamelCase. I am not gonna handle the underscores or spaces.

    If the name has two or more characters, and the first 2 or 3 characters are uppercase, it preserves the case, for instance "UI", "XML", "HTTP", etc., otherwise, it changes the first character to lowercase.

    static string ToCamelCase(string name) {
    	return Char.IsLower(name[0]) ? name
    		: name.Length == 1 ? name.ToLowerInvariant()
    		: name.Length == 2 && Char.IsUpper(name[1]) ? name
    		: name.Length > 2 && Char.IsUpper(name[1]) && Char.IsUpper(name[2]) ? name
    		: ToLower(String.Copy(name));
    
    	unsafe string ToLower(string t) {
    		fixed(char* p = t) {
    			*p = Char.ToLower(t[0]);
    		}
    		return t;
    	}
    }
  7. rpgmaker commented on Apr 5, 2021

    @rpgmaker
    Owner

    Thanks for the solution. I can see how it can integrate with space and underscore.

  8. wmjordan commented on Apr 5, 2021

    @wmjordan
    ContributorAuthor

    Perhaps we can check whether the string contains underscore or space, if so, jump to the previous slow path.

  9. wmjordan commented on Apr 5, 2021

    @wmjordan
    ContributorAuthor

    I just rechecked the source code, and saw the logic of property name writing in WritePropertiesFor. The ToCamelCase is only called in this place.

    name = propertyName
    If setting useCamelCase
     name = ToCamelCase(name)
    else
     just use the name variable

    We can furthermore optimize this to pre-calculate the camel-cased string, and store it to the dynamic assembly, then load the const string according to the corresponding setting like the following pseudo-code.

    name = setting useCamelCase ? loadPrecalculatedCamelCaseName : propertyName

    Since the camel-cased names are calculated only once in the above solution, there is little impact to the performance. Therefore, your existing function and mine in previous post won't make too much difference.

    Here is the code to replace the original lines in NetJSON.cs.

    var useCamelCaseLabel = il.DefineLabel();
    var endOfCaseLabel = il.DefineLabel();
    
    il.Emit(OpCodes.Ldloc, camelCasing);
    il.Emit(OpCodes.Brtrue_S, useCamelCaseLabel); // use short jump to save some bytes
    
    il.Emit(OpCodes.Ldstr, name); // use original name
    il.Emit(OpCodes.Stloc, nameLocal);
    il.Emit(OpCodes.Br_S, endOfCaseLabel);
    
    il.MarkLabel(useCamelCaseLabel);
    il.Emit(OpCodes.Ldstr, ToCamelCase(name)); // precalculate the camelCase name
    il.Emit(OpCodes.Stloc, nameLocal);
    
    il.MarkLabel(endOfCaseLabel);
  10. rpgmaker commented on Apr 5, 2021

    @rpgmaker
    Owner

    The only difference will be startup time for the first call there after it will be same. Thanks for the input so far. Could you still make this into a pull request? And could you run a simple test showing the difference in start up vs current?

    Thanks,

  11. wmjordan commented on Apr 5, 2021

    @wmjordan
    ContributorAuthor

    The only difference will be startup time for the first call there after it will be same.

    It is not. The existing code calls ToCamelCase each time when writing any property if the setting enables using camel case. Consequently it consumes more CPU time and generates more garbage for GC to collect.

    My previous post turns the camel-cased property names into IL const strings, so the ToCamelCase will only be called when the IL is generated. We can furthermore optimize the code to compare the result of ToCamelCase and the name and eliminate the check of camelCase setting if the names are the same (input name is already camel-cased).

    var useCamelCaseLabel = il.DefineLabel();
    var endOfCaseLabel = il.DefineLabel();
    var camelName = ToCamelCase(name);
    
    if (camelName != name) {
      il.Emit(OpCodes.Ldloc, camelCasing);
      il.Emit(OpCodes.Brtrue_S, useCamelCaseLabel); // use short jump to save some bytes
    
      il.Emit(OpCodes.Ldstr, name); // use original name
      il.Emit(OpCodes.Stloc, nameLocal);
      il.Emit(OpCodes.Br_S, endOfCaseLabel);
    
      il.MarkLabel(useCamelCaseLabel);
      il.Emit(OpCodes.Ldstr, camelName); // load the camelCase name from a const string, instead of calling ToCamelCase
      il.Emit(OpCodes.Stloc, nameLocal);
    
      il.MarkLabel(endOfCaseLabel);
    }
    else {
      il.Emit(OpCodes.Ldstr, name); // use original name
      il.Emit(OpCodes.Stloc, nameLocal);
    }

    Could you still make this into a pull request?

    Please replace the existing code with the above code. And remove this line. The rest stuff, for instance, the ToCamelCase function can be left there unchanged.

  12. rpgmaker commented on Apr 5, 2021

    @rpgmaker
    Owner

    It will if cache the result of camel case using either of our implementation was what I referring to.

    Without the caching, you are right it will cost more than just the startup.

  13. wmjordan commented on Apr 5, 2021

    @wmjordan
    ContributorAuthor

    In the existing code, Emit(OpCodes.Ldstr, name) will cache the property name anyway.

    If we are to enable runtime renaming of property names, we can create a type with string fields which holds property names and we read from those fields instead, when writing out property names.

  14. rpgmaker commented on Apr 20, 2021

    @rpgmaker
    Owner

    I will see what I can do regarding the caching logic. Was busy in the past week.

  15. rpgmaker commented on May 18, 2021

    @rpgmaker
    Owner

    I will try to get to this soon. Been busy with other stuff.

  16. rpgmaker commented on Mar 8, 2023

    @rpgmaker
    Owner

    @wmjordan , do we still need to make this change?

  17. wmjordan commented on Mar 8, 2023

    @wmjordan
    ContributorAuthor

    I am sorry to be too busy with my own projects too :P

    Jump to this issue and use the code to pre-calculate field names if you want to further improve the performance.

  18. wmjordan commented on Mar 8, 2023

    @wmjordan
    ContributorAuthor

    Once the field names are pre-calculated, the performance impact of ToLowerCase will be very minor.

  19. rpgmaker commented on Mar 8, 2023

    @rpgmaker
    Owner

    Awesome. Thanks for the clarification.

  20. rpgmaker commented on Feb 25, 2024

    @rpgmaker
    Owner

    @wmjordan, do we still want to make this changes?

  21. wmjordan commented on Feb 25, 2024

    @wmjordan
    ContributorAuthor

    Hi, I am too busy with my stuff. I have lost track with this project already.
    The code had been presented above. You just need to replace the line with copy and paste.

  22. rpgmaker commented on Feb 25, 2024

    @rpgmaker
    Owner

    Np. I will take care of it. Let me know if I can contribute anything to your projects. Thanks again for the tip.

  23. Repository owner deleted a comment from tj-appsudo on Feb 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions