-
Notifications
You must be signed in to change notification settings - Fork 544
Optimize string replacements in camel/kebab case transformations #940
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #940 +/- ##
==========================================
+ Coverage 85.37% 85.73% +0.36%
==========================================
Files 75 78 +3
Lines 2386 2468 +82
Branches 174 175 +1
==========================================
+ Hits 2037 2116 +79
- Misses 349 352 +3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thanks, and thanks especially for the benchmark! One comment but otherwise LGTM.
val default: Configuration = Configuration(Predef.identity, Predef.identity, false, None) | ||
val basePattern: Pattern = Pattern.compile("([A-Z]+)([A-Z][a-z])") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you make these private? They shouldn't be needed outside of the implementations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are fast implementations of enforcing transformations for snake-case, kebab-case, and camelCase: https://github.com/plokhotnyuk/jsoniter-scala/blob/master/jsoniter-scala-macros/src/main/scala/com/github/plokhotnyuk/jsoniter_scala/macros/JsonCodecMaker.scala#L52-L98
Please, fill free to adopt them for your requirements.
@plokhotnyuk fast implementations are indeed much faster:
The thing is that they produce slightly different outputs, and I'm not sure it fits your needs: // plokhotnyuk implementation
Configuration.snakeCaseTransformation("SnakeCase")
Configuration.snakeCaseTransformation("snakeCase")
Configuration.snakeCaseTransformation("snakecase")
Configuration.snakeCaseTransformation("snake_case")
Configuration.snakeCaseTransformation("snake-case")
Configuration.snakeCaseTransformation("SNAKECASE")
Configuration.snakeCaseTransformation("")
Configuration.snakeCaseTransformation("sNaKeCaSe")
Configuration.snakeCaseTransformation("SNAKEcase")
Configuration.snakeCaseTransformation("SnakecasE")
//res0: String = _snake_case
//res1: String = snake_case
//res2: String = snakecase
//res3: String = snake_case
//res4: String = snake_case
//res5: String = snakecase
//res6: String =
//res7: String = s_na_ke_ca_se
//res8: String = snak_ecase
//res9: String = _snakecas_e
javaRegexSnakeCaseTransformation("SnakeCase")
javaRegexSnakeCaseTransformation("snakeCase")
javaRegexSnakeCaseTransformation("snakecase")
javaRegexSnakeCaseTransformation("snake_case")
javaRegexSnakeCaseTransformation("snake-case")
javaRegexSnakeCaseTransformation("SNAKECASE")
javaRegexSnakeCaseTransformation("")
javaRegexSnakeCaseTransformation("sNaKeCaSe")
javaRegexSnakeCaseTransformation("SNAKEcase")
javaRegexSnakeCaseTransformation("SnakecasE")
//res10: String = snake_case
//res11: String = snake_case
//res12: String = snakecase
//res13: String = snake_case
//res14: String = snake-case
//res15: String = snakecase
//res16: String =
//res17: String = s_na_ke_ca_se
//res18: String = snak_ecase
//res19: String = snakecas_e
// plokhotnyuk implementation
Configuration.kebabCaseTransformation("KnakeCase")
Configuration.kebabCaseTransformation("kebabCase")
Configuration.kebabCaseTransformation("kebabcase")
Configuration.kebabCaseTransformation("kebab_case")
Configuration.kebabCaseTransformation("kebab-case")
Configuration.kebabCaseTransformation("KEBABCASE")
Configuration.kebabCaseTransformation("")
Configuration.kebabCaseTransformation("kEbAbCaSe")
Configuration.kebabCaseTransformation("KEBABcase")
Configuration.kebabCaseTransformation("KebabcasE")
//res20: String = -knake-case
//res21: String = kebab-case
//res22: String = kebabcase
//res23: String = kebab-case
//res24: String = kebab-case
//res25: String = kebabcase
//res26: String =
//res27: String = k-eb-ab-ca-se
//res28: String = keba-bcase
//res29: String = -kebabcas-e
javaRegexKebabCaseTransformation("KnakeCase")
javaRegexKebabCaseTransformation("kebabCase")
javaRegexKebabCaseTransformation("kebabcase")
javaRegexKebabCaseTransformation("kebab_case")
javaRegexKebabCaseTransformation("kebab-case")
javaRegexKebabCaseTransformation("KEBABCASE")
javaRegexKebabCaseTransformation("")
javaRegexKebabCaseTransformation("kEbAbCaSe")
javaRegexKebabCaseTransformation("KEBABcase")
javaRegexKebabCaseTransformation("KebabcasE")
//res30: String = knake-case
//res31: String = kebab-case
//res32: String = kebabcase
//res33: String = kebab_case
//res34: String = kebab-case
//res35: String = kebabcase
//res36: String =
//res37: String = k-eb-ab-ca-se
//res38: String = keba-bcase
//res39: String = kebabcas-e Let me know how you want me to proceed? |
@waiter-melon thank you for your feedback! The leading I fixed it by the following commit: |
Could we discard faster implementation and just pre-compilation of regular expressions? We should include this in the next release I guess. |
0fe82e1
to
60ea793
Compare
Thanks, @waiter-melon—these commits will make it into 0.11.0 via #940. |
Pre-compiling regular expressions drastically improves camel-case and kebab-case transformations.
Here a benchmark focused on
snakeCase
function.Before:
[info] KeyCasingBenchmark.snakeCase thrpt 10 829556.926 ± 59956.575 ops/s
After:
[info] KeyCasingBenchmark.snakeCase thrpt 10 1861369.701 ± 52228.706 ops/s