D11487: optimization of TextLineData::attribute

[Milian Wolff]

mwolff added a comment.


  In D11487#230791 <https://phabricator.kde.org/D11487#230791>, @jtamate wrote:
  
  > In D11487#230755 <https://phabricator.kde.org/D11487#230755>, @mwolff wrote:
  >
  > > 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 should give you a much better view of where the CPU cycles are spent.
  >
  >
  > One of the thinks I like about using callgrind, well, kcachegrind, is that I have the tree of calls to one method, that I don't have in perf report.
  >  As I said, I've been unable to compile hotspot yet, because I don't have the KF5 devel libraries installed from the distro, and I do not know how to specify an alternative directory in hotspsot cmake.
  
  
  Huh, just specify `-DCMAKE_INSTALL_PREFIX` to the same path where you install the KF5 devel packages to, as a simple fix. Or try to set `CMAKE_LIBRARY_PATH`, CMAKE_INCLUDE_PATH` and `CMAKE_PREFIX_PATH` env vars. Or well, just use the appimage?
  
  > I'll try again with std::lower_bound, I'll take a look at folding, but I was unable to make it work last time.
  >  There is a big difference between lower_bound and this implementation: lower_bound does a < checking, while this does a <=.
  
  Do a lower_bound and then increment from there, or maybe try to use upper_bound? I'm not acquainted with the exact semantics you are looking for, but in general: Don't reinvent the wheel, but rather try to aggregate existing building blocks.

REPOSITORY
  R39 KTextEditor

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

To: jtamate, #frameworks, #kate
Cc: mwolff, cullmann, michaelh, kevinapavew, ngraham, demsking, sars, dhaumann
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180321/d70012e9/attachment.html>