D17241: WIP:Disable highlighting after 512 characters on a line.

2019-06-17 Thread Christoph Cullmann
cullmann added a comment.


  Hmm, would a partially highlighted line really preferable?

REPOSITORY
  R39 KTextEditor

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

To: cullmann, vkrause, dhaumann, mwolff, sars
Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, LeGast00n, 
domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann


Re: KInit - Current state and benchmarks

2019-06-17 Thread Albert Astals Cid
El dilluns, 17 de juny de 2019, a les 11:56:15 CEST, David Edmundson va 
escriure:
> Results:
> (Showing the median out of 5 runs on a mid range SSD desktop)

Are we sure it's fair to assume people have SSD? our of the 4 laptops i own, 
only 2 have SSD.

Do you think it's worth me trying in one of the two that don't have SSD?

Cheers,
  Albert




D17241: WIP:Disable highlighting after 512 characters on a line.

2019-06-17 Thread Milian Wolff
mwolff added a comment.


  I personally dislike this too.

INLINE COMMENTS

> katerenderer.cpp:408
> +auto  = renderRanges.pushNewRange();
>  for (int i = 0; i < al.count(); ++i)
>  if (al[i].length > 0 && al[i].attributeValue > 0) {

couldn't the al.count be limited here? if I understand the patch correctly, we 
throw out all highlighting if we encounter too many ranges. but could we 
instead just skip the items that are beyond the limit?

REPOSITORY
  R39 KTextEditor

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

To: cullmann, vkrause, dhaumann, mwolff, sars
Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, LeGast00n, 
domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann


Re: KInit - Current state and benchmarks

2019-06-17 Thread David Edmundson
> Which libraries are covered by this mechanism nowadays? The impact is of
> course bigger the more of the dependencies of the applications are already
> loaded. When this was developed this was a small amount of relatively large Qt
> and kdelibs libraries. I'm wondering if the current subset is still relevant,
> both from Qt (e.g. thinking about QML/Qt Quick) and KF5?
>

>From what I can tell:

implicitly linked to kdeinit:
QtBase
QtGui
Crash
I18n
ConfigCore
WindowSystem

explicitly added at runtime:
KIOCore
Parts
Plasma

and all the dependencies thereof.
Note libplasma doesn't link QML/Qtquick


D19996: WIP Add a global test for insecure http: URLs used in code or documentation

2019-06-17 Thread Volker Krause
vkrause added a comment.


  In D19996#480571 , @kossebau wrote:
  
  > Any chance this could not be done by abusing KDECMakeSettings.cmake as 
injection vector?
  
  
  I completely agree that this is a rather hacky approach. IMHO the challenge 
here is finding an approach that gives us very wide coverage. That's why I'm 
not too happy with e.g. an opt-in approach where we have to enable this per 
repo, even if that would be a lot cleaner from the ECM POV.
  
  It however does not need to be ECM based at all, an alternative approach 
might be an EBN-like service or dedicated CI job scanning all our repos for 
this. That would have an even wider coverage (e.g. websites and translations), 
but it would somewhat decouple results from development. Failing unit tests 
both locally and on the CI are just jumping at you much more than yet another 
static analysis result site.

REPOSITORY
  R240 Extra CMake Modules

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

To: vkrause
Cc: kossebau, winterz, knauss, cgiboudeaux, kde-frameworks-devel, 
kde-buildsystem, LeGast00n, bencreasy, michaelh, ngraham, bruns


Re: KInit - Current state and benchmarks

2019-06-17 Thread Volker Krause
Thanks for the very interesting and useful research!

On Monday, 17 June 2019 11:56:15 CEST David Edmundson wrote:
> From API.kde.org:
> >Using kdeinit to launch KDE applications makes starting a typical KDE
> >applications 2.5 times faster (100ms instead of 250ms on a P-III 500)
> Certainly sounds like a good thing.
> 
> ===The current State===
> 
> ==Plasma==
> * Apps launched from the plasma menu skip klauncher and therefore
> kinit. This was done due to the API for KRun blocking somewhere
> (Though I don't remember details)
> 
> This seems like something easily fixable if we tried.
> 
> * Processes launched on bootup (with the exception of kcminit,
> ksmserver and kded5) skip kinit. This was due to a deadlock in
> klauncher and ksmserver at the time when apps autostart moved from
> frameworks.
> 
> This deadlock is since resolved in my ksmserver splitting branches.
> 
> * But Plasma apps launched from the desktop do go through klauncher
> (and therefore kinit)! So we're not even consistent.
> 
> ==Apps==
> * The number of applications linking kinit properly under KF5 is in an
> equally sorry state.
> Dolphin does, but even core applications like Okular and Kate don't.
> 
> See
> $ find -name 'CMakeLists.txt' -print0 | xargs -0 grep
> 'kf5_add_kdeinit_executable'
> for a local list.
> 
> * It's also incompatible with flatpak/snaps/appimage, which is a rising
> trend

That might be solvable with making that a build option (and having a 
corresponding CMake macro to automate that) I guess?

> ==Other==
> Kinit is still also used for spawning KIO slaves.
> 
> ===Is it still useful?===
> We're not using it properly and we need to do something. Either fix it
> or start to phase it out officially.
> 
> Since the time of writing kinit Qt has changed a lot. linking has
> changed a bit. CPUs have changed a lot and Hard Disks have changed a
> lot.
> 
> So I wrote a benchmark tool to see if the initial speed boost claim is
> relevant: kde:scratch/davidedmundson/inittimer
> 
> My test does the following:
>  - creates a dummy (headless) wayland server
>  - spawns an app using either QProcess or sending a DBus message to
> KLauncher - times how long it takes for the first window to appear, timing
> the more important real world metric and one that includes all the
> factors.
> 
> Results:
> (Showing the median out of 5 runs on a mid range SSD desktop)
> 
> Dolphin QProcess: 348
> Dolphin Kinit: 332
> 
> KCalc   QProcess:  242
> KCalc   KInit: 232
> 
> Plasmoidviewer (patched) QProces: 622
> Plasmoidviewer (patched) KInit: 591
> 
> KWrite  QProcess: 391
> KWrite  Kinit: 390
> (unsurprisingly similar as kwrite does not have a kdeinit exec, I
> included it as it shows the others aren't false positives)

Which libraries are covered by this mechanism nowadays? The impact is of 
course bigger the more of the dependencies of the applications are already 
loaded. When this was developed this was a small amount of relatively large Qt 
and kdelibs libraries. I'm wondering if the current subset is still relevant, 
both from Qt (e.g. thinking about QML/Qt Quick) and KF5?

> ===What about memory?===
> 
> Good question. It needs a similar investigation by someone who
> understands memory...

Regarding memory impact, I think a good first approximation is the sum of the 
sizes of the .data.rel.ro sections in all covered libraries. Those are 
allocated, written to once (for relocating information), and can then be 
shared. There can be more, but this covers vtables, QMetaObject data, etc, ie. 
stuff we tend to have a lot of. Order of magnitude is QtCore 40kB, QtGui 50kB, 
QtQml 100kB, QtDeclarative 140kB, QtWidgets 170kB. I vaguely recall this to be 
much larger in some (binary) graphics drivers, not sure if that's still the 
case though.

Hm, something that might also be worth looking into is if we can load kinit 
with the equivalent of RTLD_NOW in dlopen, so that all .got section entries 
are resolved already (making those sections fully shareable as well), and more 
importantly avoiding the symbol lookup to be done on-demand multiple times. 
Not sure if that's worth it on desktop, but for Plasma Mobile this could be 
relevant. That would probably be also my general conclusion for the entire 
topic.

Regards,
Volker

> ===Conclusion===
> 
> My test implies there /is/ a genuine saving with kinit !
> However it's far from the claimed 2.5 times faster, it is less than
> 1.1 times faster. Saving up to 30ms.



signature.asc
Description: This is a digitally signed message part.


D21813: RFC: Consider adding more padding to the desktop theme

2019-06-17 Thread Filip Fila
filipf added a comment.


  In D21813#481152 , @martinsotirov 
wrote:
  
  > I think Plasma needs a whole lot more padding all over the place, not just 
in the notifications, but I guess there must be some kind of compromise with 
all the people who like more cramped UIs.
  
  
  And in this case it would be easier if we implemented the changes in the 
desktop theme rather than in code because those wanting more cramped UIs could 
just use a different desktop theme (fork an old version e.g.).

REPOSITORY
  R242 Plasma Framework (Library)

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

To: filipf, #plasma, #vdg, ngraham, broulik
Cc: martinsotirov, abetts, ndavis, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns


Re: KInit - Current state and benchmarks

2019-06-17 Thread Christoph Cullmann

Hi,

On 2019-06-17 11:56, David Edmundson wrote:

From API.kde.org:
Using kdeinit to launch KDE applications makes starting a typical KDE 
applications 2.5 times faster (100ms instead of 250ms on a P-III 500)


Certainly sounds like a good thing.

===The current State===

==Plasma==
* Apps launched from the plasma menu skip klauncher and therefore
kinit. This was done due to the API for KRun blocking somewhere
(Though I don't remember details)

This seems like something easily fixable if we tried.

* Processes launched on bootup (with the exception of kcminit,
ksmserver and kded5) skip kinit. This was due to a deadlock in
klauncher and ksmserver at the time when apps autostart moved from
frameworks.

This deadlock is since resolved in my ksmserver splitting branches.

* But Plasma apps launched from the desktop do go through klauncher
(and therefore kinit)! So we're not even consistent.

==Apps==
* The number of applications linking kinit properly under KF5 is in an
equally sorry state.
Dolphin does, but even core applications like Okular and Kate don't.
I stopped using it I guess to be consistent between all platforms 
shipping Kate.
(beside, if you profile the Kate startup, all things are > the stuff 
kdeinit saves)




See
$ find -name 'CMakeLists.txt' -print0 | xargs -0 grep
'kf5_add_kdeinit_executable'
for a local list.

* It's also incompatible with flatpak/snaps/appimage, which is a rising 
trend


==Other==
Kinit is still also used for spawning KIO slaves.

===Is it still useful?===
We're not using it properly and we need to do something. Either fix it
or start to phase it out officially.

Since the time of writing kinit Qt has changed a lot. linking has
changed a bit. CPUs have changed a lot and Hard Disks have changed a
lot.

So I wrote a benchmark tool to see if the initial speed boost claim is
relevant: kde:scratch/davidedmundson/inittimer

My test does the following:
 - creates a dummy (headless) wayland server
 - spawns an app using either QProcess or sending a DBus message to 
KLauncher

 - times how long it takes for the first window to appear, timing the
more important real world metric and one that includes all the
factors.

Results:
(Showing the median out of 5 runs on a mid range SSD desktop)

Dolphin QProcess: 348
Dolphin Kinit: 332

KCalc   QProcess:  242
KCalc   KInit: 232

Plasmoidviewer (patched) QProces: 622
Plasmoidviewer (patched) KInit: 591

KWrite  QProcess: 391
KWrite  Kinit: 390
(unsurprisingly similar as kwrite does not have a kdeinit exec, I
included it as it shows the others aren't false positives)

===What about memory?===

Good question. It needs a similar investigation by someone who
understands memory...

===Conclusion===

My test implies there /is/ a genuine saving with kinit !
However it's far from the claimed 2.5 times faster, it is less than
1.1 times faster. Saving up to 30ms.


my opinion: it's not worh the hassle.

Greetings
Christoph

--
Ignorance is bliss...
https://cullmann.io | https://kate-editor.org


D21813: RFC: Consider adding more padding to the desktop theme

2019-06-17 Thread Martin Sotirov
martinsotirov added a comment.


  I think Plasma needs a whole lot more padding all over the place, not just in 
the notifications, but I guess there must be some kind of compromise with all 
the people who like more cramped UIs.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: filipf, #plasma, #vdg, ngraham, broulik
Cc: martinsotirov, abetts, ndavis, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns


KInit - Current state and benchmarks

2019-06-17 Thread David Edmundson
>From API.kde.org:
>Using kdeinit to launch KDE applications makes starting a typical KDE 
>applications 2.5 times faster (100ms instead of 250ms on a P-III 500)

Certainly sounds like a good thing.

===The current State===

==Plasma==
* Apps launched from the plasma menu skip klauncher and therefore
kinit. This was done due to the API for KRun blocking somewhere
(Though I don't remember details)

This seems like something easily fixable if we tried.

* Processes launched on bootup (with the exception of kcminit,
ksmserver and kded5) skip kinit. This was due to a deadlock in
klauncher and ksmserver at the time when apps autostart moved from
frameworks.

This deadlock is since resolved in my ksmserver splitting branches.

* But Plasma apps launched from the desktop do go through klauncher
(and therefore kinit)! So we're not even consistent.

==Apps==
* The number of applications linking kinit properly under KF5 is in an
equally sorry state.
Dolphin does, but even core applications like Okular and Kate don't.

See
$ find -name 'CMakeLists.txt' -print0 | xargs -0 grep
'kf5_add_kdeinit_executable'
for a local list.

* It's also incompatible with flatpak/snaps/appimage, which is a rising trend

==Other==
Kinit is still also used for spawning KIO slaves.

===Is it still useful?===
We're not using it properly and we need to do something. Either fix it
or start to phase it out officially.

Since the time of writing kinit Qt has changed a lot. linking has
changed a bit. CPUs have changed a lot and Hard Disks have changed a
lot.

So I wrote a benchmark tool to see if the initial speed boost claim is
relevant: kde:scratch/davidedmundson/inittimer

My test does the following:
 - creates a dummy (headless) wayland server
 - spawns an app using either QProcess or sending a DBus message to KLauncher
 - times how long it takes for the first window to appear, timing the
more important real world metric and one that includes all the
factors.

Results:
(Showing the median out of 5 runs on a mid range SSD desktop)

Dolphin QProcess: 348
Dolphin Kinit: 332

KCalc   QProcess:  242
KCalc   KInit: 232

Plasmoidviewer (patched) QProces: 622
Plasmoidviewer (patched) KInit: 591

KWrite  QProcess: 391
KWrite  Kinit: 390
(unsurprisingly similar as kwrite does not have a kdeinit exec, I
included it as it shows the others aren't false positives)

===What about memory?===

Good question. It needs a similar investigation by someone who
understands memory...

===Conclusion===

My test implies there /is/ a genuine saving with kinit !
However it's far from the claimed 2.5 times faster, it is less than
1.1 times faster. Saving up to 30ms.


D21813: RFC: Consider adding more padding to the desktop theme

2019-06-17 Thread Andres Betts
abetts added a comment.


  +1 on visuals!

REPOSITORY
  R242 Plasma Framework (Library)

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

To: filipf, #plasma, #vdg, ngraham, broulik
Cc: abetts, ndavis, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns