Read BAMF_DESKTOP_FILE_HINT from environment
ClosedPublic

Authored by broulik on Jul 17 2019, 10:43 AM.

Details

Summary

This contains the actual name of the desktop file which might be different from the one the application got built with.

Test Plan

5.16?
Not sure if this needs to be cached. The codepath is only hit for unidentified apps but when I e.g. launch plasmoidviewer which doesn't have a desktop file it ends up calling into this 5 times (first when PID "changes"/is set, then for window icon, etc)

Chromium snap can be pinned and launched properly now

Diff Detail

Repository
R120 Plasma Workspace
Lint
Lint Skipped
Unit
Unit Tests Skipped
broulik created this revision.Jul 17 2019, 10:43 AM
Restricted Application added a project: Plasma. · View Herald TranscriptJul 17 2019, 10:43 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik requested review of this revision.Jul 17 2019, 10:43 AM
hein added a comment.Jul 18 2019, 4:22 AM

The idea and approach are good.

But disk I/O in a UI hotpath is pretty scary. I do think a cache would be a good addition.

In general I think libtm could use some improvements on its currently very coarse cache eviction scheme. It's mostly fine but there are some edge cases where we end up doing I/O on things we know we haven't changed. Unfortunately though improving that just means doing the legwork of going case-by-case and the complexity it invites ... i.e. a cache here.

(Slightly unrelated: I have the beginnings of a patch set that uses KUserFeedback to gather telemetry on how often we use which identification means and why. I have this persistent feeling that we're falling through to the suboptimal/expensive code paths way more than we should currently, and there are probably minor changes to do "higher up in the file" to fix that. One is probably the neverending woes around normalizing for reverse DNS vs. plain .desktop file names. But in general I'd love for my system to be able go give me a trend curve for "Task Manager app identification expensiveness over time" to I can see when there's new regressions and/or changes in the app landscape.)

/proc isn't disk io

This revision was not accepted when it landed; it landed in state Needs Review.Dec 4 2019, 9:49 PM
This revision was automatically updated to reflect the committed changes.