D10670: Reduce plasmashell frozen time

[Ben Cooksley]

bcooksley added a comment.


  Phabricator considers reviews approved only when all individual and group 
reviewers have approved it.
  
  In this instance, nobody approved on behalf of #frameworks 
 and @mwolff has changes requested 
still.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D10670

To: jtamate, #frameworks, #plasma, mwolff, davidedmundson
Cc: bcooksley, hein, mwolff, plasma-devel, davidedmundson, michaelh, ZrenBot, 
ngraham, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10670: Reduce plasmashell frozen time

[Eike Hein]

hein added a comment.


  I think it's because mwolff did "Requesting changes" and he didn't accept.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D10670

To: jtamate, #frameworks, #plasma, mwolff, davidedmundson
Cc: hein, mwolff, plasma-devel, davidedmundson, michaelh, ZrenBot, ngraham, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10670: Reduce plasmashell frozen time

[Jaime Torres Amate]

jtamate added a comment.


  In D10670#230938 , @hein wrote:
  
  > > This revision was not accepted when it landed; it landed in state Needs 
Review.
  >
  > Weird, David accepted it though?
  
  
  Yes. Shouldn't this be reported as a bug to phabricator? It has happened  to 
me twice.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D10670

To: jtamate, #frameworks, #plasma, mwolff, davidedmundson
Cc: hein, mwolff, plasma-devel, davidedmundson, michaelh, ZrenBot, ngraham, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10670: Reduce plasmashell frozen time

[Eike Hein]

hein added a comment.


  > This revision was not accepted when it landed; it landed in state Needs 
Review.
  
  Weird, David accepted it though?

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D10670

To: jtamate, #frameworks, #plasma, mwolff, davidedmundson
Cc: hein, mwolff, plasma-devel, davidedmundson, michaelh, ZrenBot, ngraham, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10670: Reduce plasmashell frozen time

[Jaime Torres Amate]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R242:33ddaaa23f6b: Reduce plasmashell frozen time (authored by 
jtamate).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10670?vs=27672=30138

REVISION DETAIL
  https://phabricator.kde.org/D10670

AFFECTED FILES
  src/plasma/private/effectwatcher.cpp

To: jtamate, #frameworks, #plasma, mwolff, davidedmundson
Cc: mwolff, plasma-devel, davidedmundson, michaelh, ZrenBot, ngraham, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10670: Reduce plasmashell frozen time

[Jaime Torres Amate]

jtamate added a comment.


  In D10670#210702 , @jtamate wrote:
  
  > Updated the summary and added a comment
  
  
  ping? I would like to use the same comment and commit message in 
https://phabricator.kde.org/D10669

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D10670

To: jtamate, #frameworks, #plasma, mwolff
Cc: mwolff, plasma-devel, davidedmundson, michaelh, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10670: Reduce plasmashell frozen time

[Jaime Torres Amate]

jtamate updated this revision to Diff 27672.
jtamate edited the summary of this revision.
jtamate added a comment.


  Updated the summary and added a comment

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10670?vs=27576=27672

REVISION DETAIL
  https://phabricator.kde.org/D10670

AFFECTED FILES
  src/plasma/private/effectwatcher.cpp

To: jtamate, #frameworks, #plasma, mwolff
Cc: mwolff, plasma-devel, davidedmundson, michaelh, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10670: Reduce plasmashell frozen time

[Jaime Torres Amate]

jtamate added a comment.


  In D10670#209823 , @mwolff wrote:
  
  > ok, looking at the reasoning in the other commit:
  >
  > - you need to extend the commit message here
  
  
  Will be done.
  
  > - you need to provide a comment in the code the clarifies what's going on 
here
  
  Will be done.
  
  > in general, I don't see how such a comparison can be so costly - the real 
problem would be too many native events, no? did you maybe profile a debug 
build or something?
  
  A qstrcmp is more expensive than a single cpu instruction in i586 builds 
(debug or release), unless it uses SSE4 instruction in the x64 world.
  
  Unfortunately I haven't found what causes so many native events.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D10670

To: jtamate, #frameworks, #plasma, mwolff
Cc: mwolff, plasma-devel, davidedmundson, michaelh, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10670: Reduce plasmashell frozen time

[Milian Wolff]

mwolff added a comment.


  ok, looking at the reasoning in the other commit:
  
  - you need to extend the commit message here
  - you need to provide a comment in the code the clarifies what's going on here
  
  in general, I don't see how such a comparison can be so costly - the real 
problem would be too many native events, no? did you maybe profile a debug 
build or something?

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D10670

To: jtamate, #frameworks, #plasma, mwolff
Cc: mwolff, plasma-devel, davidedmundson, michaelh, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10670: Reduce plasmashell frozen time

[Milian Wolff]

mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.


  what? this is totally unsafe - the reinterpret_cast below shouldn't be done 
on anything that has event names other than the previously used one...

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D10670

To: jtamate, #frameworks, #plasma, mwolff
Cc: mwolff, plasma-devel, davidedmundson, michaelh, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10670: Reduce plasmashell frozen time

[Jaime Torres Amate]

jtamate created this revision.
jtamate added reviewers: Frameworks, Plasma.
Restricted Application added projects: Plasma, Frameworks.
Restricted Application added a subscriber: plasma-devel.
jtamate requested review of this revision.

REVISION SUMMARY
  Third part of https://phabricator.kde.org/D10627

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  polling (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D10670

AFFECTED FILES
  src/plasma/private/effectwatcher.cpp

To: jtamate, #frameworks, #plasma
Cc: plasma-devel, davidedmundson, michaelh, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart