Skip to content

Performance improvements to homepage recyclers and coil #2125

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.

Sign up for GitHub

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

Draft
Luna712 wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Performance improvements to homepage recyclers and coil #2125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e9333c3
Performance improvements to homepage recyclers and coil
Luna712 Nov 1, 2025
7a6cb23
Merge branch 'recloudstream:master' into perf
Luna712 Nov 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/src/main/java/com/lagradost/cloudstream3/ui/home/HomeFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,7 @@ class HomeFragment : BaseFragment<FragmentHomeBinding>(
homeViewModel.queryTextSubmit("")
}

homeMasterRecycler.setHasFixedSize(true)
Copy link
Collaborator

@fire-light42 fire-light42 Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure about this?

homeMasterRecycler.addOnScrollListener(object : RecyclerView.OnScrollListener() {
override fun onScrolled(recyclerView: RecyclerView, dx: Int, dy: Int) {
if (isLayout(PHONE)) {
Expand Down
113 changes: 43 additions & 70 deletions app/src/main/java/com/lagradost/cloudstream3/ui/home/HomeParentItemAdapter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import androidx.fragment.app.Fragment
import androidx.recyclerview.widget.LinearLayoutManager
import androidx.recyclerview.widget.RecyclerView
import androidx.viewbinding.ViewBinding
import com.lagradost.cloudstream3.HomePageList
Expand Down Expand Up @@ -48,7 +49,7 @@ open class ParentItemAdapter(
) {
companion object {
val sharedPool =
RecyclerView.RecycledViewPool().apply { this.setMaxRecycledViews(CONTENT, 4) }
RecyclerView.RecycledViewPool().apply { this.setMaxRecycledViews(CONTENT, 8) }
Copy link
Collaborator

@fire-light42 fire-light42 Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why this needs to be 8? This is the parent view for each row, not the items themself.

Copy link
Contributor Author

@Luna712 Luna712 Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because sometimes there are many rows. That was the intent anyway.

}

data class ParentItemHolder(val binding: ViewBinding) : ViewHolderState<Bundle>(binding) {
Expand Down Expand Up @@ -89,79 +90,51 @@ open class ParentItemAdapter(
) {
val startFocus = R.id.nav_rail_view
val endFocus = FOCUS_SELF
val binding = holder.view
if (binding !is HomepageParentBinding) return
val binding = holder.view as? HomepageParentBinding ?: return
val info = item.list
binding.apply {
val currentAdapter = homeChildRecyclerview.adapter as? HomeChildItemAdapter
if (currentAdapter == null) {
homeChildRecyclerview.setRecycledViewPool(HomeChildItemAdapter.sharedPool)
homeChildRecyclerview.adapter = HomeChildItemAdapter(
id = id + position + 100,
clickCallback = clickCallback,
nextFocusUp = homeChildRecyclerview.nextFocusUpId,
nextFocusDown = homeChildRecyclerview.nextFocusDownId,
).apply {
isHorizontal = info.isHorizontalImages
hasNext = item.hasNext
submitList(item.list.list)
}
} else {
currentAdapter.apply {
isHorizontal = info.isHorizontalImages
hasNext = item.hasNext
this.clickCallback = this@ParentItemAdapter.clickCallback
nextFocusUp = homeChildRecyclerview.nextFocusUpId
nextFocusDown = homeChildRecyclerview.nextFocusDownId
submitIncomparableList(item.list.list)
}
}

homeChildRecyclerview.setLinearListLayout(
isHorizontal = true,
nextLeft = startFocus,
nextRight = endFocus,
)
homeChildMoreInfo.text = info.name

homeChildRecyclerview.addOnScrollListener(object :
RecyclerView.OnScrollListener() {
var expandCount = 0
val name = item.list.name

override fun onScrollStateChanged(
recyclerView: RecyclerView,
newState: Int
) {
super.onScrollStateChanged(recyclerView, newState)

val adapter = recyclerView.adapter
if (adapter !is HomeChildItemAdapter) return

val count = adapter.itemCount
val hasNext = adapter.hasNext
/*println(
"scolling ${recyclerView.isRecyclerScrollable()} ${
recyclerView.canScrollHorizontally(
1
)
}"
)*/
//!recyclerView.canScrollHorizontally(1)
if (!recyclerView.isRecyclerScrollable() && hasNext && expandCount != count) {
expandCount = count
expandCallback?.invoke(name)
val currentAdapter = (binding.homeChildRecyclerview.adapter as? HomeChildItemAdapter)
?.apply {
Copy link
Collaborator

@fire-light42 fire-light42 Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You forgot to update the clickCallback, nextFocusUp and nextFocusDown. As sharedPool is shared between adapters this needs to always be set.

if (isHorizontal != info.isHorizontalImages) isHorizontal = info.isHorizontalImages
Copy link
Collaborator

@fire-light42 fire-light42 Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isHorizontal and hasNext is not an expensive operation? Why are you doing this check?

Copy link
Contributor Author

@Luna712 Luna712 Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I was randomly trying things and it ended up in there when I opened the PR. That won't be in the final version.

if (hasNext != item.hasNext) hasNext = item.hasNext
submitIncomparableList(info.list)
}
?: HomeChildItemAdapter(
id = id + position + 100,
clickCallback = clickCallback,
nextFocusUp = binding.homeChildRecyclerview.nextFocusUpId,
nextFocusDown = binding.homeChildRecyclerview.nextFocusDownId,
).also { adapter ->
adapter.isHorizontal = info.isHorizontalImages
adapter.hasNext = item.hasNext
adapter.submitList(info.list)

binding.homeChildRecyclerview.addOnScrollListener(object : RecyclerView.OnScrollListener() {
Copy link
Collaborator

@fire-light42 fire-light42 Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same problem as the You forgot to update the clickCallback

var expandCount = 0
override fun onScrollStateChanged(recyclerView: RecyclerView, newState: Int) {
super.onScrollStateChanged(recyclerView, newState)
val adapter = recyclerView.adapter as? HomeChildItemAdapter ?: return
if (!recyclerView.isRecyclerScrollable() && adapter.hasNext && expandCount != adapter.itemCount) {
expandCount = adapter.itemCount
expandCallback?.invoke(info.name)
}
}
}
})

//(recyclerView.adapter as HomeChildItemAdapter).notifyDataSetChanged()
if (isLayout(PHONE)) {
homeChildMoreInfo.setOnClickListener {
moreInfoClickCallback.invoke(item)
}
})
binding.homeChildRecyclerview.adapter = adapter
}
}

binding.homeChildRecyclerview.setRecycledViewPool(HomeChildItemAdapter.sharedPool)
Copy link
Collaborator

@fire-light42 fire-light42 Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that you should set the pool before attaching the adapter. I do not have a source for this, but I remember reading it.

binding.homeChildRecyclerview.setHasFixedSize(true)
Copy link
Collaborator

@fire-light42 fire-light42 Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please double check this. I am unsure if we can make such assumptions if we recycle the entire homeChildRecyclerview, but I have not checked.

binding.homeChildRecyclerview.isNestedScrollingEnabled = false
(binding.homeChildRecyclerview.layoutManager as? LinearLayoutManager)?.initialPrefetchItemCount = 8
binding.homeChildRecyclerview.setLinearListLayout(
isHorizontal = true,
nextLeft = startFocus,
nextRight = endFocus
)

binding.homeChildMoreInfo.text = info.name
if (isLayout(PHONE)) binding.homeChildMoreInfo.setOnClickListener { moreInfoClickCallback(item) }
}

override fun onCreateContent(parent: ViewGroup): ParentItemHolder {
Expand Down
32 changes: 20 additions & 12 deletions app/src/main/java/com/lagradost/cloudstream3/utils/ImageModuleCoil.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,24 @@ import coil3.util.DebugLogger
import com.lagradost.cloudstream3.BuildConfig
import com.lagradost.cloudstream3.USER_AGENT
import com.lagradost.cloudstream3.network.buildDefaultClient
import okhttp3.HttpUrl
import okio.Path.Companion.toOkioPath
import java.io.File
import java.nio.ByteBuffer
import kotlinx.coroutines.Dispatchers
import okhttp3.HttpUrl
import okio.Path.Companion.toOkioPath

object ImageLoader {

private const val TAG = "CoilImgLoader"

internal fun buildImageLoader(context: PlatformContext): ImageLoader = ImageLoader.Builder(context)
.crossfade(200)
.allowHardware(SDK_INT >= 28) // SDK_INT >= 28, cant use hardware bitmaps for Palette Builder
.allowHardware(SDK_INT >= 28) // SDK_INT < 28, can't use hardware bitmaps for Palette Builder
Copy link
Collaborator

@fire-light42 fire-light42 Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you misinterpreted the comment. The comment is "SDK_INT >= 28" and "can't use hardware bitmaps for Palette Builder", they are unrelated, and it does not make any sense to switch >= to <. See 3636d8e

Copy link
Contributor Author

@Luna712 Luna712 Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that was my had yeah whoops.

.diskCachePolicy(CachePolicy.ENABLED)
.memoryCachePolicy(CachePolicy.ENABLED)
.networkCachePolicy(CachePolicy.ENABLED)
.memoryCache {
MemoryCache.Builder().maxSizePercent(context, 0.1) // Use 10 % of the app's available memory for caching
MemoryCache.Builder().maxSizePercent(context, 0.1) // Use 10% of the app's available memory for caching
.build()
}
.diskCache {
Expand All @@ -52,6 +54,8 @@ object ImageLoader {
.maxSizePercent(0.04) // Use 4 % of the device's storage space for disk caching
.build()
}
.fetcherCoroutineContext(Dispatchers.IO.limitedParallelism(8))
.decoderCoroutineContext(Dispatchers.IO.limitedParallelism(3))
/** Pass interceptors with care, unnecessary passing tokens to servers
or image hosting services causes unauthorized exceptions **/
.components { add(OkHttpNetworkFetcherFactory(callFactory = { buildDefaultClient(context) })) }
Expand Down Expand Up @@ -85,25 +89,29 @@ object ImageLoader {
headers: Map<String, String>? = null,
builder: ImageRequest.Builder.() -> Unit = {} // for placeholder, error & transformations
) {
// clear image to avoid loading & flickering issue at fast scrolling (e.g, an image recycler)
this.dispose()
if (imageData == null) {
Copy link
Collaborator

@fire-light42 fire-light42 Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What? dispose stops the current request and clears the image. I do not see what advantage this brings. Moreover, will this not break if we do loadImage(url); loadImage(null) ?

Copy link
Collaborator

@fire-light42 fire-light42 Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to write

this.dispose()
this.setImageDrawable(null)

this.setImageDrawable(null)
return
}

if(imageData == null) return // Just in case
// Only dispose if a different image is requested to avoid unnecessary reloads
Copy link
Collaborator

@fire-light42 fire-light42 Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really a thing that happens? Does dispose really show up in a flamegraph?

Copy link
Contributor Author

@Luna712 Luna712 Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well the idea was to not clear and reload the image every time but only if the actual source changes but its also possible I misinterpreted the intent behind dispose here.

if (this.tag != imageData) {
this.dispose()
this.tag = imageData
}

// setImageResource is better than coil3 on resources due to attr
if(imageData is Int) {
if (imageData is Int) {
this.setImageResource(imageData)
return
}

// Use Coil's built-in load method but with our custom module & a decent USER-AGENT always
// which can be overridden by extensions.
this.load(imageData, SingletonImageLoader.get(context)) {
this.httpHeaders(NetworkHeaders.Builder().also { headerBuilder ->
httpHeaders(NetworkHeaders.Builder().also { headerBuilder ->
headerBuilder["User-Agent"] = USER_AGENT
headers?.forEach { (key, value) ->
headerBuilder[key] = value
}
headers?.forEach { (key, value) -> headerBuilder[key] = value }
}.build())

builder() // if passed
Expand Down