Conversation
|
just out of iterest: could you also test how much data is saved when opening a video detail page and when loading comments? Thank you. |
Yeah good idea. I concentrated too much on profiling/debugging that forgot I can test other things 😅 I will test it soon. Moreover it finally clicked that I can easily remove any image data noise by using |
|
@TobiGr the table is updated! Descriptions are small and they are already compressed by gzip. They don't profit from brotli at all |
|
I'd be in favor of adding brotli. I would not expect any improvements for non text-based content(images, audio, video, ...). |
So there comes mainly two questions:
I agree. |
Where do you see this? Both of these return I also do not expect a well-configured webserver to ever compress non-text content, this is what Caddy does out of the box: https://caddyserver.com/docs/caddyfile/directives/encode |
|
Thanks for your research! Given your findings, I'd also be ok with merging this PR as is. If problems come up, it will be very easy to revert.
Since all browsers since quite some time already support brotli, and they include 'br' in 'Accept-Encoding', we become less fingerprintable if we also support brotli. So I think this is not needed.
No. If it actually is a problem on some phones, I wouldn't add brotli at all, rather than add a new setting that noone will ever find out about.
I guess you meant 'br' right? https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Accept-Encoding |
Wait a bit, I finally got an opportunity to test/profile on real device. I hope api 26 is enough because google isn't certain by itself if I need api 26 or 29.
I mean that
We support old devices (android 5.0) too. Just imagine how additional 20MB of ram would look for 3-4GB device with android 10 vs 20MB of ram for device with android 5.0 and only 1GB of ram. And you know newer androids have better ART compilation strategies which improves performance (if it uses native .so library it isn't very applicable but I didn't check) so android 5.0 devices can suffer while all new feel normal. But I hope I will be able to profile it with device I got access to and give some results
Yeah |
By doing something like And then looking in the logcat output. It shows what our app actually sends and receives not what is shown in browser. I also just tried to look on it via firefox.
|
I was wrong. The device I got access to can't be used for profiling so everything returns to start position. I can't really do any further testing so it can be merged if you want |
I don't understand. Firefox desktop, Chromium desktop, Firefox mobile all send
Yeah, that's why I'm saying, if it turns out to be too slow on those devices, let's not add brotli at all. Adding an option for a technical detail nobody knows about would not make sense.
I have a Fairphone 3 which is a pretty slow device. If you share some instructions I can test myself. |
You look on it from perspective of separate independent bool toggles? I look on them from perspective of whole coupled string as identifier because even order matters here
There is plan B. Refactor branch which now targets minimum api 23 (android 6). Not big difference but who is going to use refactor on android 6? Taking into account it was unavailable for downloads for a very long time
I've never did by myself except castrated experience with waydroid so expect my instruction to be a dead end Requirements:
Setup:
Steps:
Offtopic: my xiaomi redmi 8 (which seems to belong to the same performance category) was really slow but after installing lineage os 23 (android 16) it is quite fast. Though when I firstly installed lineage 20/21 it was a downgrade in at least half of use cases but now it is like new. |
|
Huawei MediaPad AGS2
Benchmark action: update of 119 subscriptions Screenshots: Peak RAM usage is roughly 256MB on both. CPU peaks roughly 50-60% on both without much difference between them |
|
Conclusion: RAM is generally not affected at all But do we need to be so stupid to lock (all) users from this feature? Especially if benches are correct and impact on modern devices is negligible? We can just add check if api >= 26 (or whatever) and enable it for those who certainly won't be impacted? |
For us it doesn't really matter amount of cores available for us. We need only physical cores count as we use it for filtering weak devices. _SC_NPROCESSORS_CONF - amount of physical cores (android devices doesn't have extrnal cpu cores hotplug). **Used** _SC_NPROCESSORS_ONLN - amount of cores available for OS right now. Often smaller than _SC_NPROCESSORS_CONF because OS can deactivate cores for powersave Runtime.getRuntime().availableProcessors() - a core count available to jvm. Has the same limitations as _SC_NPROCESSORS_ONLN but also can be decreased because some cores are allocated to other more priority processes (?)
|
I added check based on how many cores processor have (>2) to filter out very weak devices (I am not sure if they are really gonna struggle with brotli but just want to play safely) (Only) Checking if arch is arm32 is wrong solution because "Samsung Galaxy A02S" (on which I did one of the benchmarks) is arm32 and it didn't struggle |





What is it?
refactorbranchDescription of the changes in your PR
Accept-Encoding: brotli gzipsupport to every http request for every service (so maybe it needs special handling to add it only for youtube)¯\_(ツ)_/¯Fixes the following issue(s)
APK testing
The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR. You can find more info and a video demonstration on this wiki page.
Due diligence
Footnotes
Showed numbers for clean 0.28.2 and 0.28.2 + brotli_test commit, dev branch will probably give other results especially taking into account recent messing with resource optimizations ↩
Probably it is just a measurement error and there is no difference ↩ ↩2
because of artificial limit we have to not be rate limited they can be the same ↩