search

brave / brave-browser

Brave browser for Android, iOS, Linux, macOS, Windows.
https://brave.com
Mozilla Public License 2.0
17.97k stars 2.35k forks source link

Move handling mojom::BatAdsClient from UI thread #41632

Open atuchin-m opened 1 month ago

atuchin-m commented 1 month ago

As investigated in https://github.com/brave/brave-browser/issues/41549 decoding DBTransactionInfo mojo messages happens on UI thread. In some real-world scenarios it could takes 30ms+ even on high-perf m1 machines resulting in hanging UI thread. That code need to be move from UI or replace to a one without mojom-encoding/decoding (as @aseren suggested in the thread). Image

slack thread: https://bravesoftware.slack.com/archives/C3T9S9WKD/p1728567272430889

iefremov commented 1 month ago

@tmancey perhaps this could be P2 - the user impact is pretty visible (so i assume this not merely dev concern, but a product bug, when reproduced)

bridiver commented 1 month ago

Why are we returning such a large amount of data from the DB in the first place vs paginating the results or doing aggregations in SQL?

tmancey commented 1 month ago

@iefremov because this to my knowledge only occurred after a recent migration path, we added as a P3 which will be fixed before we bump the database version again. If this has changed we can move to a P2.

tmancey commented 1 month ago

I will run some perf tests locally using chrome://tracing. Thanks

atuchin-m commented 1 month ago

The trace from the perf pre test when we have this long UI task. tmplc3ac0yr.pb.gz

Image