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
ngraham added a comment.
Is anything else needed here before we can land this?
REPOSITORY
R288 KJobWidgets
REVISION DETAIL
https://phabricator.kde.org/D9770
To: jtamate, #frameworks, mwolff
Cc: ngraham, cfeck, mwolff, broulik
ngraham edited the summary of this revision.
REPOSITORY
R288 KJobWidgets
REVISION DETAIL
https://phabricator.kde.org/D9770
To: jtamate, #frameworks, mwolff
Cc: ngraham, cfeck, mwolff, broulik
jtamate added a comment.
In https://phabricator.kde.org/D9770#189242, @mwolff wrote:
> 10 calls per second sound fine to me, that shouldn't be a big performance
issue at all.
Yes, that is what I tough, but as soon as I changed the code,
dolphin(file.so) started to copy as fast
mwolff added a comment.
10 calls per second sound fine to me, that shouldn't be a big performance
issue at all. Are you measuring performance of a debug build or of a release
build? Can you specify the exact commands you are profiling? Is the performance
better when you are using KFormat
jtamate updated this revision to Diff 25121.
jtamate added a comment.
- De-duplication of code in byteSize(double size)
Changed the include.
There is no need to change the CMakeLists.txt as
find_package(KF5CoreAddons ${KF5_DEP_VERSION} REQUIRED)
is already included.
REPOSITORY
cfeck added inline comments.
INLINE COMMENTS
> kjobtrackerformatters.cpp:26
> #include "kjobtrackerformatters_p.h"
> +#include "kformat.h"
>
Isn't KFormat from a different framework? Adjust the include to use
style, and check if CMakeLists.txt needs to be adjusted.
REPOSITORY
R288
jtamate updated this revision to Diff 25114.
jtamate added a comment.
- De-duplication of code in byteSize(double size)
In fact Milian Wolff is right.
Why so many calls to this function?
Copying 20Gb it is called 19.026 times (aproximately 10 times per second) from
mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.
Imo this should be using `KFormat::formatByteSize`.
https://api.kde.org/frameworks/kcoreaddons/html/classKFormat.html#ae7412420b70e2ca935d0ebed6770e313
And if that function
broulik added a comment.
Nice catch.
INLINE COMMENTS
> kjobtrackerformatters.cpp:29
>
> +const char* units[] {"%1 B", "%1 KiB", "%1 MiB", "%1 GiB", "%1 TiB", "%1
> PiB", "%1 EiB", "%1 ZiB", "%1 YiB"};
> +const int unitsSize = sizeof(units)/sizeof(const char *);
These aren't translated
jtamate created this revision.
jtamate added a reviewer: Frameworks.
Restricted Application added a project: Frameworks.
jtamate requested review of this revision.
REVISION SUMMARY
Copying 20Gib, using callgrind, before it used 5,2% of the cpu, and
after only 0,58%.
Applying this patch, the
11 matches
Mail list logo
