D11811: avoid Asan runtime error: shift exponent -1 is negative

2018-04-03 Thread Milian Wolff
mwolff added a comment. maybe instead make it explicit what the old code did instead? i.e. check what you get from GCC for "1 << -1" and then use that when i == 0 REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D11811 To: jtamate, #kate, #frameworks Cc: mwolff,

D7909: Add syntax support for Crystal Programming Language

2018-04-03 Thread Milian Wolff
mwolff added subscribers: cullmann, mwolff. mwolff accepted this revision as: mwolff. mwolff added a comment. we got a test file, and it seems to work for a user of the Crystal language, so +1 from my side. @dhaumann, @cullmann ? REPOSITORY R40 Kate REVISION DETAIL

D11822: Make the word/char count a global preference

2018-04-03 Thread Milian Wolff
mwolff added subscribers: cullmann, dhaumann, mwolff. mwolff accepted this revision as: mwolff. mwolff added a comment. This revision is now accepted and ready to land. I'm not using that feature, but code-wise this lgtm - but please wait for @dhaumann or @cullmann to give their +1 too

D11487: optimization of TextLineData::attribute

2018-04-03 Thread Milian Wolff
mwolff added a comment. Ah, I knew it :D I suspected missing compiler optimizations from the start... But tricky indeed for you to see! But the real question would now be: what time does it take *without* this patch for you? probably it won't be more than a minute anymore. Still, this

D11902: Add highlighting for GDB command listings and gdbinit files

2018-04-03 Thread Milian Wolff
mwolff added reviewers: vkrause, dhaumann, Kate. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D11902 To: mwolff, vkrause, dhaumann, #kate Cc: #frameworks, michaelh, ngraham

D11902: Add highlighting for GDB command listings and gdbinit files

2018-04-03 Thread Milian Wolff
mwolff created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. mwolff requested review of this revision. REVISION SUMMARY Command listings start their non-output lines with (gdb) or > for multi-line commands. gdbinit

D11183: Sonnet: don't impose the default client

2018-03-26 Thread Milian Wolff
mwolff added inline comments. INLINE COMMENTS > cullmann wrote in loader.cpp:103 > Isn't there a == lClients.constEnd() behind that does that? ah, then this should be simplified by using `std::any_of` instead REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D11183 To:

D11183: Sonnet: don't impose the default client

2018-03-26 Thread Milian Wolff
mwolff added inline comments. INLINE COMMENTS > loader.cpp:103 > +// if it does it will be an element of lClients. > +bool unknown = std::find_if(lClients.constBegin(), > lClients.constEnd(), [backend] (const Client *client) { > +return client->name() == backend;

D11487: optimization of TextLineData::attribute

2018-03-23 Thread Milian Wolff
mwolff added a comment. WTF :D Your desktop CPU has clearly a better performance than my mobile CPU, no? Is AMD really so much worse here? How can that be - I don't get it :D Anyhow, I give up trying to understand this now - thanks a lot for your repeated input Jaime! REPOSITORY R39

D11487: optimization of TextLineData::attribute

2018-03-23 Thread Milian Wolff
mwolff added a comment. @jtamate I just checked, the function is called with the same parameters for me locally. What output do you get for this: diff --git a/src/spellcheck/spellcheck.cpp b/src/spellcheck/spellcheck.cpp index 9e04d788..50e92885 100644 ---

D11487: optimization of TextLineData::attribute

2018-03-23 Thread Milian Wolff
mwolff added a comment. +1 from my side, @cullmann, @dhaumann ? I'll continue to figure out why I don't see the performance issue here REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D11487 To: jtamate, #frameworks, #kate Cc: anthonyfieroni, dhaumann, mwolff,

D11487: optimization of TextLineData::attribute

2018-03-23 Thread Milian Wolff
mwolff added a comment. In D11487#232258 , @jtamate wrote: > > One question to that though: Why do you sort/lookup by `x.offset + x.length <= p`? Note how lower_bound returns the first iterator that is _not_ going to return true. > >

D11487: optimization of TextLineData::attribute

2018-03-23 Thread Milian Wolff
mwolff added a comment. Also, what is "@mwolf solution" - I didn't provide any code, until now: diff --git a/src/spellcheck/spellcheck.cpp b/src/spellcheck/spellcheck.cpp index 9e04d788..3a97e5c5 100644 --- a/src/spellcheck/spellcheck.cpp +++ b/src/spellcheck/spellcheck.cpp

D11487: optimization of TextLineData::attribute

2018-03-23 Thread Milian Wolff
mwolff added a comment. In D11487#231656 , @jtamate wrote: > In D11487#231522 , @mwolff wrote: > > > @jtamate looking at your screenshots, it represents closely what I see locally. Most notably,

D11487: optimization of TextLineData::attribute

2018-03-22 Thread Milian Wolff
mwolff added a comment. @jtamate looking at your screenshots, it represents closely what I see locally. Most notably, there are no red underlines in your screenshots which could arise due to spell checking. Thus I really wonder why you are seeing such a big hotspot there. Try perf, or

D11487: optimization of TextLineData::attribute

2018-03-22 Thread Milian Wolff
mwolff added inline comments. INLINE COMMENTS > anthonyfieroni wrote in katetextline.cpp:214-216 > Use operator->, it's faster than operator* and operator. > > first->offset sorry, but that's simply not true at all. Stylistic wise I agree, but an optimizing compiler will generate the same

D11487: optimization of TextLineData::attribute

2018-03-21 Thread Milian Wolff
mwolff added a comment. Considering spell checking is involved - can you show a screenshot for how the file looks like for you? There shouldn't be a lot of spell checking going on, or so I hope... REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D11487 To:

D11487: optimization of TextLineData::attribute

2018-03-21 Thread Milian Wolff
mwolff added a comment. When I open your fake file in kwrite, then raise the line length limit and wait for the file to be rendered (which takes quite some time!), perf does not show any performance issues with attribute for me. It only corresponds to ~1.3% of the CPU cycles.

D11487: optimization of TextLineData::attribute

2018-03-21 Thread Milian Wolff
mwolff added a comment. In D11487#230913 , @jtamate wrote: > In D11487#230895 , @mwolff wrote: > > > Looking at `TextLineData::addAttribute`, it doesn't seem to sort the data - how can you be sure

D11487: optimization of TextLineData::attribute

2018-03-21 Thread Milian Wolff
mwolff added a subscriber: dhaumann. mwolff added a comment. Looking at `TextLineData::addAttribute`, it doesn't seem to sort the data - how can you be sure that the attributes list really is sorted by `offset + length`? I think this should be done manually there, or at least asserted. The

D11487: optimization of TextLineData::attribute

2018-03-21 Thread Milian Wolff
mwolff added a comment. In D11487#230791 , @jtamate wrote: > In D11487#230755 , @mwolff wrote: > > > yes, definitely don't roll your own lower_bound - use the STL provided one. Are you really

D11487: optimization of TextLineData::attribute

2018-03-21 Thread Milian Wolff
mwolff added a comment. yes, definitely don't roll your own lower_bound - use the STL provided one. Are you really compiling in release mode while measuring this? Also, I can only repeat myself in saying that you shouldn't use callgrind for performance measurements anymore, perf/hotspot

D11487: optimization of TextLineData::attribute

2018-03-20 Thread Milian Wolff
mwolff added a comment. can you publish the test file and how you measured it? Then I can test it with perf too, to confirm the impact. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D11487 To: jtamate, #frameworks, #kate Cc: mwolff, cullmann, michaelh,

D11282: less expensive findByUrl in KCoreDirListerCache

2018-03-15 Thread Milian Wolff
mwolff added a comment. I'm not into the code base, just adding some comments to ensure everything is taken into account - maybe your initial attempt was better after all... INLINE COMMENTS > kcoredirlister.cpp:852 > +if (refresh) { > +(*it).refresh(); >

D10742: get rid of the raw KFileItem pointers in KCoreDirListerCache

2018-03-15 Thread Milian Wolff
mwolff added a comment. no real review, just want to mention that you'll still have heap allocations for every item - now once per container it is in (due to QList and QSet and sizeof the KFileItem) REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10742 To: jtamate,

Re: Would it be good if I became baloo's maintainer?

2018-03-15 Thread Milian Wolff
ated into KDE as baloo? > * What should be my general attitude towards that task? -- Milian Wolff m...@milianw.de http://milianw.de

D11282: less expensive findByUrl in KCoreDirListerCache

2018-03-13 Thread Milian Wolff
mwolff added a comment. have you considered using a hash map instead? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D11282 To: jtamate, #frameworks, dfaure Cc: mwolff, michaelh

D11132: Avoid an asan runtime error

2018-03-08 Thread Milian Wolff
mwolff added a comment. still accepted, fell free to submit REPOSITORY R302 KIconThemes REVISION DETAIL https://phabricator.kde.org/D11132 To: jtamate, #frameworks, mwolff Cc: mwolff, apol, anthonyfieroni, michaelh

D11132: Avoid an asan runtime error

2018-03-08 Thread Milian Wolff
mwolff accepted this revision. mwolff added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > kiconeffect.cpp:44 > +KIconEffectPrivate() > + : effect{{}} > + , value{{}} it may be a good idea to add a comment that links to

D10712: balooctl monitor: Resume to wait for service

2018-03-07 Thread Milian Wolff
mwolff accepted this revision. This revision is now accepted and ready to land. REPOSITORY R293 Baloo BRANCH resume-wait (branched from master) REVISION DETAIL https://phabricator.kde.org/D10712 To: michaelh, #baloo, #frameworks, dfaure, alexeymin, mwolff Cc: mwolff, ashaposhnikov,

D10712: balooctl monitor: Resume to wait for service

2018-03-07 Thread Milian Wolff
mwolff requested changes to this revision. mwolff added a comment. This revision now requires changes to proceed. some minor comments, otherwise lgtm INLINE COMMENTS > monitorcommand.cpp:45 > +connect(m_dbusServiceWatcher, ::serviceUnregistered, > [this]() { > +m_err <<

Re: Code in peace or How to lint a complete repository?

2018-03-01 Thread Milian Wolff
plied by the Qt project. I additionally setup my Git pre- commit hooks via [1] and use git-clang-format a lot. [1]: https://github.com/BelledonneCommunications/ortp/blob/master/.git-pre-commit Bye -- Milian Wolff m...@milianw.de http://milianw.de signature.asc Description: This is a digitally signed message part.

D10860: Do not allow to configure separator actions via context menu

2018-02-26 Thread Milian Wolff
This revision was automatically updated to reflect the committed changes. Closed by commit R263:67c59d7bb56e: Do not allow to configure separator actions via context menu (authored by mwolff). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D10860?vs=28108=28110#toc REPOSITORY R263

D10860: Do not allow to configure separator actions via context menu

2018-02-26 Thread Milian Wolff
mwolff updated this revision to Diff 28108. mwolff added a comment. rebase REPOSITORY R263 KXmlGui CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10860?vs=28106=28108 BRANCH master REVISION DETAIL https://phabricator.kde.org/D10860 AFFECTED FILES

D9675: Don't show context menu menu if right-clicking outside

2018-02-26 Thread Milian Wolff
mwolff accepted this revision. This revision is now accepted and ready to land. REPOSITORY R263 KXmlGui REVISION DETAIL https://phabricator.kde.org/D9675 To: broulik, #frameworks, dfaure, mwolff Cc: elvisangelaccio, wbauer, michaelh

D10860: Do not allow to configure separator actions via context menu

2018-02-26 Thread Milian Wolff
mwolff created this revision. mwolff added a reviewer: dfaure. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. mwolff requested review of this revision. REVISION SUMMARY It's not useful to add shortcuts for separators, nor to add them

D10670: Reduce plasmashell frozen time

2018-02-19 Thread 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

D10670: Reduce plasmashell frozen time

2018-02-19 Thread 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

D7968: Forward QComboBox signals instead of QComboBox lineedit signals

2018-02-15 Thread Milian Wolff
mwolff added a comment. In D7968#206500 , @whiting wrote: > Hey all, this change breaks Kompare https://bugs.kde.org/show_bug.cgi?id=390024 which watches a KUrlRequester's textChanged signal to update a button's enabled state. We are not seeing

D9840: Tentative patch to reduce I/O overhead of plasmashell when copying files

2018-02-14 Thread Milian Wolff
mwolff added a comment. I guess this can be abandoned now that the real culprit was found, no? REPOSITORY R309 KService REVISION DETAIL https://phabricator.kde.org/D9840 To: jtamate, #frameworks, dfaure Cc: mwolff, ngraham, mmustac, michaelh

D10257: KUrlMimeData: fix handling of PreferLocalUrls

2018-02-06 Thread Milian Wolff
mwolff added inline comments. INLINE COMMENTS > kurlmimedata.cpp:67 > +QList uris; > +const QByteArray ba = > mimeData->data(QString::fromLatin1(s_kdeUriListMime)); > +// Code from qmimedata.cpp future cleanup: remove all the `QString::fromLatin1(s_...)` in this file with a call

D10024: Add supportedSchemes feature

2018-02-06 Thread Milian Wolff
This revision was automatically updated to reflect the committed changes. Closed by commit R241:6ab218dba91f: Add supportedSchemes feature (authored by mwolff). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D10024?vs=25891=26632#toc REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D10045: remote: don't create entries with empty names

2018-02-06 Thread Milian Wolff
This revision was automatically updated to reflect the committed changes. Closed by commit R241:4d153df7359c: remote: dont create entries with empty names (authored by mwolff). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10045?vs=26203=26633 REVISION DETAIL

D10024: Add supportedSchemes feature

2018-02-06 Thread Milian Wolff
mwolff added a comment. I've committed this now. If someone tells me how to add KIOSK support, I can add that later on as needed REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D10024 To: mwolff, #plasma, dfaure, mart Cc: ngraham, mart, apol,

D10124: Faster simplejob start

2018-02-02 Thread Milian Wolff
mwolff added a comment. can you test and make sure that q and slave actually get destroyed? If that is the case, then the slotobj should get destroyed too. If not then this is a bug in Qt, which would make me wonder... Do you have a selfcompiled Qt, or one shipped by your distro? There was

D10124: Faster simplejob start

2018-02-02 Thread Milian Wolff
mwolff added a comment. only that one, not the others? strange, why should that be the case? are you sure `q` and `slave` get destroyed? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10124 To: jtamate, #frameworks, mwolff Cc: mwolff, broulik, ngraham, anthonyfieroni,

Re: KWalletManager improvement

2018-01-30 Thread Milian Wolff
What you describe sounds like one goal that Valentin originally had for KSecrets. So polishing that and making it usable is probably the right way? I have no idea what the state of that is though. Bye -- Milian Wolff m...@milianw.de http://milianw.de

Re: save and restore the geometry of KMainWindow

2018-01-30 Thread Milian Wolff
in > > http://doc.qt.io/qt-5/application-windows.html#window-geometry ? > > > > Can somebody point to an application doing this correctly? This works for me: https://github.com/KDAB/hotspot/blob/4d1177d1631902dce1dd82f53553e97a7544b1fa/ src/mainwindow.cpp#L162 What window manager are you using? -- Milian Wolff m...@milianw.de http://milianw.de

Re: save and restore the geometry of KMainWindow

2018-01-30 Thread Milian Wolff
in > > http://doc.qt.io/qt-5/application-windows.html#window-geometry ? > > > > Can somebody point to an application doing this correctly? This works for me: https://github.com/KDAB/hotspot/blob/4d1177d1631902dce1dd82f53553e97a7544b1fa/ src/mainwindow.cpp#L162 What window manager are you using? -- Milian Wolff m...@milianw.de http://milianw.de

D10124: Faster simplejob start

2018-01-30 Thread Milian Wolff
mwolff requested changes to this revision. mwolff added a comment. This revision now requires changes to proceed. the crash may be due to missing context, the three-arg connect shouldn't ever be used imo INLINE COMMENTS > simplejob.cpp:141 > > -q->connect(slave, SIGNAL(connected()), >

D10155: Use the much faster urls() method from QMimeData

2018-01-30 Thread Milian Wolff
mwolff added inline comments. INLINE COMMENTS > kurlmimedata.cpp:74 > } > QByteArray ba = mimeData->data(QString::fromLatin1(firstMimeType)); > if (ba.isEmpty()) { if `firstMimeType == "text/uri-list"` (see above, `PreferLocalUrls`), it's still going to be slow, no? Should this

D10045: remote: don't create entries with empty names

2018-01-30 Thread Milian Wolff
mwolff retitled this revision from "Don't assert on empty names" to "remote: don't create entries with empty names". mwolff edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10045 To: mwolff, dfaure Cc: dhaumann, #frameworks, michaelh

D10045: Don't assert on empty names

2018-01-30 Thread Milian Wolff
mwolff added inline comments. INLINE COMMENTS > remoteimpl.cpp:56 > > -const QStringList filenames = dir.entryList(QDir::Files | > QDir::Readable); > +const QStringList filenames = > dir.entryList({QStringLiteral("*.desktop")}, > +

D10045: Don't assert on empty names

2018-01-30 Thread Milian Wolff
mwolff updated this revision to Diff 26203. mwolff added a comment. filter out empty entries from remote:/ KIO slave REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10045?vs=25808=26203 BRANCH master REVISION DETAIL https://phabricator.kde.org/D10045

D10045: Don't assert on empty names

2018-01-30 Thread Milian Wolff
mwolff added a comment. OK, that's not enough. It seems like the file can sometimes be read while it's still empty: 7.098 debug: RemoteImpl::createEntry[/home/milian/projects/kf5/src/frameworks/kio/src/ioslaves/remote/remoteimpl.cpp:188]: UDS_NAME "" from

D10045: Don't assert on empty names

2018-01-30 Thread Milian Wolff
mwolff added a comment. 41.919 warning: KConfigIniBackend::parseConfig[/home/milian/projects/kf5/src/frameworks/kconfig/src/core/kconfigini.cpp:253]: "KConfigIni: In file /home/milian/.local/share/remoteview/yxcvyxcv.desktop.lock, line 1: " Invalid entry (missing '=') 41.919 warning:

D10172: KRun: allow executing "add network folder" without confirmation prompt

2018-01-30 Thread Milian Wolff
mwolff accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D10172 To: dfaure, mwolff, dhaumann Cc: elvisangelaccio, ngraham, #dolphin, #frameworks, michaelh

D10024: Add supportedSchemes feature

2018-01-30 Thread Milian Wolff
mwolff added a comment. @apol any hint what to do about KIOSK? any resources I should have a look at? REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D10024 To: mwolff, #plasma, dfaure, mart Cc: ngraham, mart, apol, plasma-devel, #frameworks, michaelh,

D10024: Add supportedSchemes feature

2018-01-24 Thread Milian Wolff
mwolff added a comment. In https://phabricator.kde.org/D10024#194708, @mart wrote: > ship it with the extra check Aleix noted Can you guys explain me more what would be required for KIOSK support? REPOSITORY R241 KIO BRANCH master REVISION DETAIL

D10024: Add supportedSchemes feature

2018-01-24 Thread Milian Wolff
mwolff updated this revision to Diff 25891. mwolff added a comment. extend apidox, mention empty list == everything allowed REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10024?vs=25764=25891 BRANCH master REVISION DETAIL

D10045: Don't assert on empty names

2018-01-24 Thread Milian Wolff
mwolff added a comment. In https://phabricator.kde.org/D10045#195097, @dfaure wrote: > Good point, done. I did the following: - in Dolphin, go to remote:/ - add network folder - allow execution of this script (separate usability issue) - webdav, then fill out the form

D10045: Don't assert on empty names

2018-01-23 Thread Milian Wolff
mwolff created this revision. mwolff added a reviewer: dfaure. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. mwolff requested review of this revision. REVISION SUMMARY This removes an assertion I just hit. Note how the code below

D10024: Add supportedSchemes feature

2018-01-23 Thread Milian Wolff
mwolff added a comment. In https://phabricator.kde.org/D10024#194431, @apol wrote: > Makes sense I guess. > > Sounds like it would be useful to set this through kiosk, would that work? Probably, but I have zero experience about kiosk. I'd have to read the defaults from a

D10024: Add supportedSchemes feature

2018-01-22 Thread Milian Wolff
mwolff added a dependent revision: D10025: Use KFileWidget::setSupportedSchemes when available. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10024 To: mwolff, #plasma, dfaure Cc: plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg,

D10024: Add supportedSchemes feature

2018-01-22 Thread Milian Wolff
mwolff created this revision. mwolff added reviewers: Plasma, dfaure. Restricted Application added projects: Plasma, Frameworks. Restricted Application added subscribers: Frameworks, plasma-devel. mwolff requested review of this revision. REVISION SUMMARY This addds a whitelist of supported URL

D9824: Optimize inotify KDirWatch backend: map inotify wd to Entry

2018-01-22 Thread Milian Wolff
This revision was automatically updated to reflect the committed changes. Closed by commit R244:78e9ffa1844e: Optimize inotify KDirWatch backend: map inotify wd to Entry (authored by mwolff). REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE

D9824: Optimize inotify KDirWatch backend: map inotify wd to Entry

2018-01-22 Thread Milian Wolff
mwolff added a comment. In https://phabricator.kde.org/D9824#193095, @dfaure wrote: > This patch fixes a O(n) performance issue in the inotify backend using a cache, which makes KDirWatch *better* suited for applications that use KDirWatch heavily. Clearly a step in the right direction.

Re: kdesu - console output and password keeping

2018-01-22 Thread Milian Wolff
On Sunday, January 21, 2018 10:07:20 PM CET Albert Astals Cid wrote: > El dijous, 18 de gener de 2018, a les 10:44:04 CET, Milian Wolff va escriure: > > Hey all, > > > > We've recently integrated kdesu into hotspot [1] to be able to record > > profile data as root. Thi

D9766: Use at least the requested width for the argument hint tree

2018-01-18 Thread Milian Wolff
mwolff abandoned this revision. mwolff added a comment. this was comitted already, closing it again REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D9766 To: mwolff, #kdevelop, #kate Cc: dhaumann, kfunk, #frameworks, michaelh, kevinapavew, ngraham, demsking,

D9822: Add benchmarks for KDirWatch

2018-01-18 Thread Milian Wolff
This revision was automatically updated to reflect the committed changes. Closed by commit R244:421e85344db7: Add benchmarks for KDirWatch (authored by mwolff). REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9822?vs=25368=25583 REVISION DETAIL

D9820: Verify that the file was opened

2018-01-18 Thread Milian Wolff
This revision was automatically updated to reflect the committed changes. Closed by commit R244:311a702fa070: Verify that the file was opened (authored by mwolff). REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9820?vs=25167=25581 REVISION DETAIL

D9823: Optimize: use QMetaObject::invokeMethod with functor

2018-01-18 Thread Milian Wolff
This revision was automatically updated to reflect the committed changes. Closed by commit R244:16e8cf1e6607: Optimize: use QMetaObject::invokeMethod with functor (authored by mwolff). REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9823?vs=25366=25584

D9821: Use QSignalSpy::wait instead of QTest::wait where possible

2018-01-18 Thread Milian Wolff
This revision was automatically updated to reflect the committed changes. Closed by commit R244:5d91ecc2fb68: Use QSignalSpy::wait instead of QTest::wait where possible (authored by mwolff). REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE

D9819: Verify that the path is valid and writable

2018-01-18 Thread Milian Wolff
This revision was automatically updated to reflect the committed changes. Closed by commit R244:5fa1aaa2a47b: Verify that the path is valid and writable (authored by mwolff). REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9819?vs=25166=25580 REVISION

kdesu - console output and password keeping

2018-01-18 Thread Milian Wolff
;...more args...> -- \ runuser -u $non-root-user -- \ $userapp $userargs kdesu -u root -t --attach $winId -- chown $non-root-user:$non-root-group $outputfile This triggers two password prompts, which is unfortunate I think. Thanks [1]: https://github.com/KDAB/hotspot -- Milian Wolff m...

D9766: Use at least the requested width for the argument hint tree

2018-01-16 Thread Milian Wolff
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 R39:0d0284e8f8d2: Use at least the requested width for the argument hint tree (authored by mwolff). REPOSITORY R39

D9824: Optimize inotify KDirWatch backend: map inotify wd to Entry

2018-01-16 Thread Milian Wolff
mwolff added a comment. In https://phabricator.kde.org/D9824#191748, @rjvbb wrote: > So please do measure the impact of your fix on performance with the QFSW backend. It's zero, since this patch only touches the inotify backend. REPOSITORY R244 KCoreAddons REVISION DETAIL

D9824: Optimize inotify KDirWatch backend: map inotify wd to Entry

2018-01-16 Thread Milian Wolff
mwolff added a comment. Yeah, I agree. Let's fix it. But I won't write mega patches like you seem to prefer. I try to keep things as minimal as possible. And I also can't give you an ETA or guarantee on when I'll fix the rest. This patch solves one big issues, and the added benchmark lies

D9766: Use at least the requested width for the argument hint tree

2018-01-16 Thread Milian Wolff
mwolff added a subscriber: kfunk. mwolff added a comment. ping? @kfunk maybe? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D9766 To: mwolff, #kdevelop, #kate Cc: kfunk, #frameworks, michaelh, kevinapavew, ngraham, demsking, cullmann, sars, dhaumann

D9824: Optimize inotify KDirWatch backend: map inotify wd to Entry

2018-01-16 Thread Milian Wolff
mwolff added a comment. @rjvbb why did you request changes to proceed with this patch? The fact that there are more issues in KDirWatch does not mean we should hold up this approach. Also, please note how the fundamental issue described here affects more backends beside inotify, but

D9824: Optimize inotify KDirWatch backend: map inotify wd to Entry

2018-01-16 Thread Milian Wolff
mwolff updated this revision to Diff 25441. mwolff added a comment. use QHash::insert instead of operator[] REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9824?vs=25171=25441 REVISION DETAIL https://phabricator.kde.org/D9824 AFFECTED FILES

D9888: Make kdesu work when PWD is /usr/bin

2018-01-16 Thread Milian Wolff
This revision was automatically updated to reflect the committed changes. Closed by commit R299:86954e039463: Make kdesu work when PWD is /usr/bin (authored by mwolff). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D9888?vs=25395=25440#toc REPOSITORY R299 KDESu CHANGES SINCE LAST

D9888: Make kdesu work when PWD is /usr/bin

2018-01-15 Thread Milian Wolff
mwolff updated this revision to Diff 25395. mwolff retitled this revision from "Let kdesu work when __PATH_SU or __PATH_SUDO are not defined" to "Make kdesu work when PWD is /usr/bin". mwolff edited the summary of this revision. mwolff added a subscriber: sitter. mwolff added a comment.

D9888: Let kdesu work when __PATH_SU or __PATH_SUDO are not defined

2018-01-15 Thread Milian Wolff
mwolff added a comment. FTR: [13:35] how the hell can kdesu ever work? https://phabricator.kde.org/D9888 WTF [13:35] does anyone know if I'm missing something? [13:35] googling for these macros doesn't show me anything obvious either [13:53] milian: dead code I'd say.

D9888: Let kdesu work when __PATH_SU or __PATH_SUDO are not defined

2018-01-15 Thread Milian Wolff
mwolff edited the summary of this revision. REPOSITORY R299 KDESu REVISION DETAIL https://phabricator.kde.org/D9888 To: mwolff, dfaure, mpyne Cc: #frameworks

D9888: Let kdesu work when __PATH_SU or __PATH_SUDO are not defined

2018-01-15 Thread Milian Wolff
mwolff created this revision. mwolff added reviewers: dfaure, mpyne. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. mwolff requested review of this revision. REVISION SUMMARY I quite frankly don't understand how this code can ever

D9770: Optimization of byteSize(double size)

2018-01-15 Thread Milian Wolff
mwolff accepted this revision. mwolff added a comment. This revision is now accepted and ready to land. - do not ever profile a debug build, the results are completely bogus - do not use callgrind, use perf that said, I'm OK with this patch but the commit message should not talk about

D9822: Add benchmarks for KDirWatch

2018-01-15 Thread Milian Wolff
mwolff updated this revision to Diff 25368. mwolff edited the summary of this revision. mwolff added a comment. format results on phab REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9822?vs=25367=25368 REVISION DETAIL

D9822: Add benchmarks for KDirWatch

2018-01-15 Thread Milian Wolff
mwolff updated this revision to Diff 25367. mwolff edited the summary of this revision. mwolff added a comment. fixup commit message REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9822?vs=25169=25367 REVISION DETAIL https://phabricator.kde.org/D9822

D9824: Optimize inotify KDirWatch backend: map inotify wd to Entry

2018-01-15 Thread Milian Wolff
mwolff added a comment. 150KB is not a lot of memory REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D9824 To: mwolff, dfaure, rjvbb, #kdevelop Cc: #frameworks

D9821: Use QSignalSpy::wait instead of QTest::wait where possible

2018-01-15 Thread Milian Wolff
mwolff added inline comments. INLINE COMMENTS > dfaure wrote in kdirwatch_unittest.cpp:256 > but then the whole while loop could be removed too, it served the same > purpose as what QSignalSpy::wait does. no, since wait waits for one signal, this loop waits for up to `expected` signals. We

D9823: Optimize: use QMetaObject::invokeMethod with functor

2018-01-15 Thread Milian Wolff
mwolff updated this revision to Diff 25366. mwolff added a comment. thanks dfaure ;-) REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9823?vs=25172=25366 BRANCH master REVISION DETAIL https://phabricator.kde.org/D9823 AFFECTED FILES

D9823: Optimize: use QMetaObject::invokeMethod with functor

2018-01-11 Thread Milian Wolff
mwolff added a comment. https://codereview.qt-project.org/#/c/216497/ REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D9823 To: mwolff, dfaure, apol Cc: #frameworks

D9823: Optimize: use QMetaObject::invokeMethod with functor

2018-01-11 Thread Milian Wolff
mwolff added a comment. cleaner, yes. but also much slower. contrary to the other code-paths, the `QTimer::singleShot` taking a functor is not optimized (yet?) for `timeout == 0`...F5638638: Screenshot_20180111_170829.png REPOSITORY R244 KCoreAddons

D9823: Optimize: use QMetaObject::invokeMethod with functor

2018-01-11 Thread Milian Wolff
mwolff updated this revision to Diff 25172. mwolff added a comment. This revision is now accepted and ready to land. make compile against older Qt REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9823?vs=25170=25172 REVISION DETAIL

D9823: Optimize: use QMetaObject::invokeMethod with functor

2018-01-11 Thread Milian Wolff
mwolff planned changes to this revision. mwolff added a comment. requires 5.10, so I can't commit this as-is... REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D9823 To: mwolff, dfaure, apol Cc: #frameworks

D9824: Optimize inotify KDirWatch backend: map inotify wd to Entry

2018-01-11 Thread Milian Wolff
mwolff added a comment. @rjvbb this patch just shows that KDirWatch has tons of performance issues that need to be fixed here. Please do run the new benchmarks on your benchmark with your backend, profile them, optimize them. REPOSITORY R244 KCoreAddons REVISION DETAIL

D9824: Optimize inotify KDirWatch backend: map inotify wd to Entry

2018-01-11 Thread Milian Wolff
mwolff added reviewers: rjvbb, KDevelop. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D9824 To: mwolff, dfaure, rjvbb, #kdevelop Cc: #frameworks

D9824: Optimize inotify KDirWatch backend: map inotify wd to Entry

2018-01-11 Thread Milian Wolff
mwolff created this revision. mwolff added a reviewer: dfaure. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. mwolff requested review of this revision. REVISION SUMMARY This greatly reduces the on-CPU time of the benchNotifyWatcher.

D9819: Verify that the path is valid and writable

2018-01-11 Thread Milian Wolff
mwolff created this revision. mwolff added a reviewer: dfaure. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. mwolff requested review of this revision. REVISION SUMMARY This prevents strange errors when trying to run the test when

<    1   2   3   4   5   6   7   8   9   >