bug(YouTube): Fix Litho battery drain (Testers Needed)

[0xrxL]

Bug description

Based on a recent issue report on inotia00's repository (which I start to reproduce 2/3 months ago):

inotia00/ReVanced_Extended#2898

YouTube start to drain battery after posting comments on any video. This lead to visible app stuttering and device overheating.

Error logs

Many GC Freed spammed in the logcat

Solution

I tried to figure out how to make a fix to prevent it, by saving some memory allocation profiles and tracking the stack calls of problematic classes. After different trying, this is what I was able to understand from this situation:

• Battery drain is caused by a memory leak in one Litho class, with many GC Freed spammed in the logcat.
• This class manage elements like the bottom action buttons bar in fullscreen mode and other minor things.
• All Litho classes are managed by a ThreadPoolExecutor and some Thread objects created subsequently.

The fix - or the workaround, to be more clear - implementation is trivial enough:

• Set both minimum and maximum ThreadPoolExecutor threads amount to 1 and allowCoreThreadTimeout to true (it's set to false as default).
• Set the ThreadPoolExecutor timeout to 1 and the Threads priority to 1 as well.

That's all...

I'm trying this workaround by three days, and cannot reproduce the battery drain anymore after posting comments (probably because the problematic thread is purged fast enough to prevent the memory leak).

Additional context

To find the classes: Perform a search by the string "ComponentLayoutThread", to find the threads classes (are two: one with obfuscated name and another named ComponentTree), and by reference allowCoreThreadTimeout (there is only one in the entire source code) to find the ThreadPoolExecutor.

Note: In the aformentioned class with obfuscated name, which creates the threads, there is an integer field initialized in the init method that must be set to 1. This field is readed by another obfuscated class that create ls further Litho related threads.

Acknowledgements

• I have checked all open and closed bug reports and this is not a duplicate.
• I have chosen an appropriate title.
• All requested information has been provided properly.

[LisoUseInAIKyrios]

Thread priority of 1 is min priority, 10 is max priority. It looks like it already uses max thread priority (20.25.23 class Lfyc;)

If I'm looking at the right class (YT 20.25.23 Lfye;) it uses a thread keep alive time of 1 second which should be no different than 0 seconds.

[LisoUseInAIKyrios]

The app has some logic to pick between using a fixed thread pool of size 1, 2, or 3 threads. (20.25.23 class Laira;)
It seems to use:
3 threads -> Device has over 6 cores and more than 6GB of memory
2 threads -> Device has over 6 cores and less than 6GB of memory
1 thread - > Device has less than 6 cores

It appears that changing the fixed pool size to 1 fixes the watch history shelf sometimes incorrectly hiding itself (it doesn't appear to be ReVanced that is incorrectly hiding the shelf, as it's not a patch causing it to hide).

[LisoUseInAIKyrios]

If the thread pool is fixed to 1 thread, I don't know if there's any benefit to allowing core threads to timeout.

Allowing a timeout might make the UI less responsive since any UI activity requires launching a new thread instead of using a thread in the pool. The only way I can think of that keeping threads around could leak memory is if objects are stored in thread local instances. And as far as I can tell the only thread local usage in YT is very small objects such as string formatters, UI paths, small fixed size buffers, and other small fixed size objects that should never be a memory issue.

[cyberboh]

The app has some logic to pick between using a fixed thread pool of size 1, 2, or 3 threads. (20.25.23 class Laira;) It seems to use: 3 threads -> Device has over 6 cores and more than 6GB of memory 2 threads -> Device has over 6 cores and less than 6GB of memory 1 thread - > Device has less than 6 cores

It appears that changing the fixed pool size to 1 fixes the watch history shelf sometimes incorrectly hiding itself (it doesn't appear to be ReVanced that is incorrectly hiding the shelf, as it's not a patch causing it to hide).

Does v22 patches branch work with 20.25.23?

[LisoUseInAIKyrios]

Litho filtering is still partially broken with 20.22+ (buffer filtering fails for most cases), but everything else works.

I'm about to make a test PR with just this change that can be used with 20.12

[cyberboh]

Litho filtering is still partially broken with 20.22+ (buffer filtering fails for most cases), but everything else works.

I'm about to make a test PR with just this change that can be used with 20.12

How test throughly? I already built v22 patches with recent commits.

[LisoUseInAIKyrios]

I just made the change this morning. It's only been tested a few minutes but I can already tell it fixes the incorrectly hidden watch history bug. Stock YT already uses 1, 2, or 3 threads, so the change only forces the 1 thread executor that is already used on some slower devices.

[cyberboh]

3 threads -> Device has over 6 cores and more than 6GB of memory
2 threads -> Device has over 6 cores and less than 6GB of memory
1 thread - > Device has less than 6 cores

Are these automatically selected? How about to make them as an options? Because newer devices usually use modern Chipset, so user can select the option based on their device

[LisoUseInAIKyrios]

The 3/2/1 criteria I listed is what YT is already using to pick the thread pool size.

It could be an option to pick one of those, but if using 1 thread fixes bugs in unpatched YT then it probably should always be using 1.

[0xrxL]

Thread priority of 1 is min priority, 10 is max priority. It looks like it already uses max thread priority (20.25.23 class Lfyc;)

If I'm looking at the right class (YT 20.25.23 Lfye;) it uses a thread keep alive time of 1 second which should be no different than 0 seconds.

Oh...so you didn't noticed any difference? In this case leave it to 10. 🤷‍♂️

[0xrxL]

I just made the change this morning. It's only been tested a few minutes but I can already tell it fixes the incorrectly hidden watch history bug. Stock YT already uses 1, 2, or 3 threads, so the change only forces the 1 thread executor that is already used on some slower devices.

Great Google devs deserve a great modded client. 😂

[LisoUseInAIKyrios]

Anyone who can reproduce the comment box issue can try patching with the linked pull request artifacts.

But the PR can be merged without verification of that, since it fixes the watch history issue and using a single thread pool is a normal use case of YT.