ComposeGears/Valkyrie

[Enhancement] Lazy properties in code output

vladmircan opened this issue · 11 comments

Description
Instead of generating a custom singleton pattern for every icon, we could use the built-in lazy functionality.
For example:

val ArrowLeft: ImageVector
    get() {
        if (_ArrowLeft != null) {
            return _ArrowLeft!!
        }
        _ArrowLeft = ImageVector.Builder(....).build()

        return _ArrowLeft!!
    }

private var _ArrowLeft: ImageVector? = null

becomes

val ArrowLeft: ImageVector by lazy {
     ImageVector.Builder(....).build()
}

A minor thing to be sure but it does:

  1. Reduce indentation by one level
  2. Make the singleton pattern clearer
  3. Reduce the lines of code by 8 / per file which across something like 300 icons becomes significant

P.S. Love the plugin!

Hello, thanks for idea 🙂
I thought about this option and want to add in next releases.

One point, I think better to add LazyThreadSafetyMode.NONE) to avoid unnecessary synchronization in this place.
Smth like this:

val ArrowLeft: ImageVector by lazy(LazyThreadSafetyMode.NONE) {
     ImageVector.Builder(....).build()
}

Even better, looking forward to it!
Feel free to close this issue unless you want to keep it around as a reminder.

Will keep open to track progress 🙂

I believe this is not ideal because lazy init will increase memory consumption by keeping a reference to the initializer and the lazy property delegate wrapper that in this case will serve no purpose other than looking pretty. If Google wanted to use lazy for their icons, they would have already done that, but they didn't.

Yes, it is one of the reason why backing property is default output in plugin 🙂
Lazy block is just an alternative, but devs should be careful with this.

@vladmircan feature implemented in latest nightly build: https://github.com/ComposeGears/Valkyrie/releases/tag/Nightly
could you please check on your project?

@egorikftp It works, great job! Love that you added a toggle to choose between code outputs.
For my information and others that may stumble upon this thread, what is the expected additional memory footprint per image?

GPT has other opinion about lazy block:

Memory Differences Analysis

Memory Overhead:

  1. Custom Lazy Initialization: The first approach has a slight overhead due to the nullable _Icon property. Each access involves a check and an additional property holder.

  2. lazy Delegate Initialization: It allocates additional memory through its internal mechanisms, but generally more optimized and compact compared to manually handling the lazy initialization.

The actual memory difference between these approaches is likely minimal and may not significantly impact performance, especially if the initialization code is lightweight and the application doesn't heavily rely on these icons.
However, lazily initializing via lazy is usually more idiomatic, concise, and potentially more performant because it avoids the manual null check and reduces boilerplate code.

Conclusions:

  • Maintainability and Readability: Using lazy is generally more readable and maintainable.

  • Memory Efficiency: lazy is optimized for memory and skips explicit null checks.

  • Thread Safety: Both approaches, as provided, are not thread-safe. For thread safety, use LazyThreadSafetyMode.SYNCHRONIZED as needed.

@egorikftp Something to be aware of for sure, thanks for posting the analysis!

GPT is not entirely correct.
If we use non-thread-safe lazy, the additional footprint will be an additional class header and instance allocation + a dynamic mutable reference. The lazy internally also uses an if check to determine if the variable is initialized. The null value is only using a few cpu cycles for dereferencing and memory pointer comparison.

At this point this discussion is mostly nitpicking and overengineering, but I think the best solution is to just let the user choose the approach they want to use.