https://kotlinlang.org logo
#codereview
Title
# codereview
m

Mark

02/23/2022, 8:01 AM
Copy code
/**
 * usage: val regex by "foo".lazyRegex(RegexOption.IGNORE_CASE)
 * instead of:
 * val regex by lazy { "foo".toRegex(RegexOption.IGNORE_CASE) }
 * or:
 * val regex = "foo".toRegex(RegexOption.IGNORE_CASE)
 */
fun String.lazyRegex(option: RegexOption? = null): Lazy<Regex> =
    lazy {
        if (option == null)
            toRegex()
        else
            toRegex(option)
    }
So here, if you already have a bunch of properties like:
Copy code
val regex = "foo".toRegex()
you can very easily convert to:
Copy code
val regex by "foo".lazyRegex()
p

Paul Woitaschek

02/23/2022, 8:43 AM
And what is the benefit of this?
☝️ 2
m

Mark

02/23/2022, 8:52 AM
Unnecessary pattern compilation at app startup
A big disadvantage is there is no longer a warning (Android Studio) when the pattern has an issue. Is there any way to do that?
r

Roukanken

02/23/2022, 9:03 AM
I would assume it's because it even losses the highlighting right? Lemme look for injection that's supposed to handle this
in JetBrain IDEs there is a way to automatically inject languages to strings and similar
if you open the Kotlin regex pattern you'll notice smth like
Copy code
+ kotlinParameter().ofFunction(0, kotlinFunction().withName("Regex").definedInClass("kotlin.text.Regex"))
+ receiver().ofFunction(kotlinFunction().withName("toRegex").withReceiver("kotlin.String").definedInPackage("kotlin.text"))
+ receiver().ofFunction(kotlinFunction().withName("toPattern").withReceiver("kotlin.String").definedInPackage("kotlin.text"))
in "Places patterns"
should be simple enough to extend to also include your custom function
eg, adding
+ receiver().ofFunction(kotlinFunction().withName("toLazyRegex").withReceiver("kotlin.String").definedInPackage("......"))
m

Mark

02/23/2022, 9:07 AM
Nice, and there’s something satisfying about using a regex to detect a regex call 🙂 I’ll give it a shot!
r

Roukanken

02/23/2022, 9:08 AM
it's not regex, it's just rules (tho afaik there are ways to use regex, JS SQL detection uses regex iirc to detect it)
m

Mark

02/23/2022, 9:10 AM
Indeed, that makes more sense than a regex-based solution anyway, which would likely lead to false-positives
p

Paul Woitaschek

02/23/2022, 9:14 AM
Have you measured the performance impact of this? This smells like premature optimization.
2
m

Mark

02/23/2022, 9:26 AM
I agree, it does smell like that, and, no, I haven’t measured the performance impact. I don’t have experience with making such measurements. Is it easy to, say, find out the time taken for all calls to
.toRegex()
and
.toRegex(RegexOption)
during startup? I have about 1000+ Regex properties and some of the patterns are very complex, so I’d be interested to measure anyway.
@Roukanken worked a treat, thanks very much. I didn’t know that could be configured in that way.
👍 1
p

Paul Woitaschek

02/23/2022, 9:29 AM
https://firebase.google.com/docs/perf-mon/get-started-android Probably this is what you are looking for
👍 1
m

Mark

02/23/2022, 9:31 AM
Isn’t there a way to do it with the performance tools in AS?
This will probably cover your case better
My expectation is that 1000 regex parsing won't be noticeable at all.
👍 1
m

Mark

02/23/2022, 9:35 AM
Surely depends how complex the patterns are?
p

Paul Woitaschek

02/23/2022, 9:36 AM
Let's find out :) I'd say compared to any file IO it's peanuts
m

Mark

02/23/2022, 9:37 AM
Unless the pattern’s being read from disk 😜
p

Paul Woitaschek

02/23/2022, 9:39 AM
Then your written function doesn't handle that properly 🧌
e

ephemient

02/23/2022, 9:53 AM
lazy involves kproperty initialization at startup and synchronization on every operation. don't assume it's cheaper. measure.
👍 1
💯 1
m

Mark

02/23/2022, 11:14 AM
Turns out the vast majority of classes holding the properties are not loaded during app startup anyway, so only 7ms (on my S10e) is spent creating Regexes during startup. Approx 0.5ms per Regex on average. So, yup, no need for this code here, but happy to have learnt about language injections and using the AS profiler. Thanks!
👍 1
p

Paul Woitaschek

02/23/2022, 11:22 AM
That’s not nothing at least
You could still go with a memory cache, i.e sth like this: https://gist.github.com/PaulWoitaschek/d3c84e0ae8e91cf9ceebb05286bd9fbe
👌 1
That will give you syntax highlighting, you have all your regex logic grouped together and its lazy
m

Mark

02/23/2022, 11:26 AM
Interesting thanks. Although I guess that’s not thread-safe?
p

Paul Woitaschek

02/23/2022, 11:27 AM
No its not but it there is no need for that. The costs of synchronization is probably more expensive than having one regex created twice
m

Mark

02/23/2022, 11:30 AM
Thread-safety is important in my case. If I was to use that pattern, I’d rather put a
by lazy
regex property on the enum class.
p

Paul Woitaschek

02/23/2022, 11:31 AM
Why is that important?
a

Adam Powell

02/23/2022, 4:25 PM
I am incredibly curious what the use case for 1000 statically declared regex patterns is 🙂
😄 1
p

Paul Woitaschek

02/23/2022, 4:26 PM
Probably a list of allowed passwords 🧌
🙈 2
m

Mark

02/24/2022, 12:59 AM
@Adam Powell It’s a dictionary app, and not a single one deals with passwords 😜 . Most are not statically declared. Many are just instance properties. But, yes, if Regex instantiation is much cheaper than I originally thought, then perhaps only the patterns need to be statically declared (although there are several patterns which are dynamically generated).
e

ephemient

02/24/2022, 1:07 AM
also, at least on JVM, statics aren't initialized until the class is loaded, which happens on demand. so a top-level property in its own file or object is equivalent to a
lazy
, except that the JVM handles synchronization on initialization, doesn't require synchronization on subsequent accesses, and doesn't require Kotlin to generate all the KProperty/delegate stuff.
☝️ 1
👍 2
m

Mark

02/24/2022, 1:09 AM
Thanks. Yes, this was confirmed by my testing. So there are far less use cases for
lazy
than one might originally have thought.
@ephemient Aren’t top-level properties/functions tied to the package not the file?
e

ephemient

02/24/2022, 1:18 AM
Kotlin compiles all the top-level elements (functions and properties) of each Kotlin file to a separate Java class (unless you use
@file:JvmMultifileClass
)
👍 1
it's just Kotlin magic that makes them appear to be package-level
m

Mark

02/24/2022, 1:27 AM
@ephemient would you say it makes sense, by default, to declare Regex properties at the top-level?
e

ephemient

02/24/2022, 1:28 AM
if they're known at compile time, then I'd put them in objects (companion or other singleton) or at the top level
1
👍 1
m

Mark

02/24/2022, 2:04 AM
It would be good to have a default approach, because it’s not feasible to measure each case. So the default would be to declare properties (top-level, singleton etc) unless profiling suggests to use lazy. However, how about something like this where a regex (character class) is constructed from (about 100) keys in a map (assume the keys do not need to be escaped):
Copy code
fooMap
    .keys
    .joinToString(prefix = "[", separator = "", postfix = "]")
    .toRegex()
Would you default to 1️⃣ lazy or 2️⃣ standard property?
2️⃣ 2
5 Views