perf The module flattening function can reduce GC using MutableSet

[KAMO030]

The module flattening function can reduce GC using MutableSet

If "tailrec" is not necessary, it can be more concise and efficient

no tailrec + use MutableSet Fun：

fun flatModules(modules: List<Module>, newModules: MutableSet<Module> = mutableSetOf()): Set<Module> {
    return newModules.apply {
        modules.forEach {
            this += it
            flatModules(it.includedModules, this)
        }
    }
}

[1250422131]

When we use the newModules.add(head) method, although it reduces the need to create a new collection, we are actually modifying the original collection newModules. This means that we are changing the state of the original collection by adding the new element head to it.

In contrast, using the newModules + head operator creates a new collection object that contains all the elements of the original collection newModules along with the new element head. This operation does not modify the original collection; instead, it returns a new collection object.

By using newModules + head, we adhere to the principle of not modifying the original collection and avoid potential side effects. The benefit of doing this is that we can create a new collection object in each iteration without affecting the newModules collection passed as input. This helps ensure the reliability and maintainability of the code.

Therefore, using the newModules + head operator instead of the newModules.add(head) method follows the principles of functional programming, avoids side effects, and maintains code clarity and readability.

However, newModules.add(head) is also fine. It does indeed avoid creating new objects and saves memory overhead. We just need to make sure to communicate this to future developers, so they won't be confused when working with this code.

[XLZJBFZ]

We can use queue to traverse modules. At the beginning, we add the modules to the queue. Each time, we take a module out of the queue and add it to the set, while also add the included modules from this module to the queue until the queue is empty. Using queue instead of recursion can reduce GC while not changing the original module set.

fun flatten(modules: List<Module>, newModules: Set<Module> = emptySet()): Set<Module> {
    val queue: Queue<Module> = LinkedList(modules)
    val flattenedModules = newModules.toMutableSet()
    while (queue.isNotEmpty()) {
        queue.poll().run {
            this.includedModules.forEach {
                queue.offer(it)
            }
            flattenedModules += this
        }
    }
    return flattenedModules.toSet()
}

[KAMO030]

Intersting, any statistics behind this?

203 modules with a statistical deep of 10 ：

no tailrec + use MutableSet:286
Before use Set:9495
After use MutableSet:6488

I think without 'tailrec' and using 'MutableSet' is a better choice
Because both solve the consumption caused by "head. importedModules+tail" and "newModules+head"
of course, provided that 'tailrec' is not necessary
and Removing the default value "newModules" of function input parameters can prevent external unsafe calls

fun flatten(modules: List<Module>): Set<Module> {
    fun flat(modules: List<Module>, newModules: MutableSet<Module>){
        modules.forEach{
            newModules += it
            flat(it.includedModules,newModules)
        }
    }
    return mutableSetOf<Module>().apply { flat(modules,this) }
}

[arnaudgiuliani]

in this case, I would tend to say that tail rec is not mandatory. The call graph is not big enough in memory I guess 🤔
Even an app with few hundreds of modules, the recursion is not super deep.

Instead of overriding the current function, we can write a second one and use it for a moment. We need to observe the gain. Some of you could help make test on a beta version of Koin 3.5?

[KAMO030]

in this case, I would tend to say that tail rec is not mandatory. The call graph is not big enough in memory I guess 🤔 Even an app with few hundreds of modules, the recursion is not super deep.

Instead of overriding the current function, we can write a second one and use it for a moment. We need to observe the gain. Some of you could help make test on a beta version of Koin 3.5?

No problem, Do I need to recommit the 'no tailrec' version of the code?

[arnaudgiuliani]

yep, update your proposal with best no tailrec approach 👍
thanks

[KAMO030]

yep, update your proposal with best no tailrec approach 👍
thanks

ok，is updated

[arnaudgiuliani]

PR #1643 has been merged. Can you update your PR a last time?

[KAMO030]

PR #1643 has been merged. Can you update your PR a last time?

How should I update PR

[arnaudgiuliani]

that's ok, I thought there were remaining conflicts 👍

[arnaudgiuliani]

Good job, and thanks all!