[KProtocolInfoFactory] Don't clear cache if it had just been built
ClosedPublic

Authored by hein on Mar 7 2018, 7:58 AM.

Details

Summary

KProtocolInfoFactory::findProtocol clears its cache when requested an unknown protocol in the hope of finding it due to an outdated cache. However, when the cache had just been created, there's no point in clearing and recreating it right away

Test Plan

Mitigates the effects of D9094 somewhat (takes 4.5ms instead of 9ms on startup for me now)

Diff Detail

Repository
R241 KIO
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
broulik created this revision.Mar 7 2018, 7:58 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptMar 7 2018, 7:58 AM
broulik requested review of this revision.Mar 7 2018, 7:58 AM
hein added a comment.Mar 7 2018, 10:26 AM

Good idea, but every time one of those state tracking bools get copied my mind screams "BAD PATTERN". It's not the fault of your patch but I don't like how this cache handling is written - it'd be cleaner if fillCache had a return value for "I actually did work" and if the meaning of m_allProtocolsLoaded got inverted to being a dirty bit, so fillCache() itself can take care of the cache eviction code (qDeleteAll etc.) if dirty is set. Doing this instead creates a "now every time you want to change things you have to read site first and copy some boiler plate" situation.

hein commandeered this revision.Mar 7 2018, 11:13 AM
hein edited reviewers, added: broulik; removed: hein.

Commandeering as agreed upon with Kai, new rev in a moment.

hein updated this revision to Diff 28916.Mar 7 2018, 11:15 AM

Slightly cleaner approach.

hein added a reviewer: dfaure.Mar 10 2018, 2:44 PM
dfaure accepted this revision.Mar 10 2018, 7:33 PM
This revision is now accepted and ready to land.Mar 10 2018, 7:33 PM
This revision was automatically updated to reflect the committed changes.