D17289: KDevelop/Shell: set dedicated TMPDIR

René J.V. Bertin noreply at phabricator.kde.org
Mon Jan 28 10:52:38 GMT 2019


rjvbb added a comment.


  >   we use CXTranslationUnit_CreatePreambleOnFirstParse to get code completion results fast. otherwise the first code completion request would create the preamble, which felt much worse
  
  Shall we keep that discussion to D18551 <https://phabricator.kde.org/D18551>?
  
  >   that's ~50ish code locations... Quite a lot :-/
  
  Erm, yes, so many that it begs the question if "this is worth it"... Is it?
  
  I only looked at places where the existing env. profile feature is used. There were already enough of those that I didn't feel like changing them all potentially for nothing, but I don't think there were this many.
  
  Why would we not limit the change to those locations, or in other words, why is the selected env. profile not used for every process KDevelop spawns?
  
  Would it be feasible to reduce the footprint of this overhaul by using a wrapper around writableLocation(QSP::TempLocation) instead? I'd have to check in how many locations libclang reads TMPDIR and so if it's feasible to change that variable temporarily around the parser's entrypoints into libclang.
  
  (replying by email)
  
  >> +#if defined(Q_OS_UNIX)
  >>  +    auto tmpLocation = QStandardPaths::writableLocation(QStandardPaths::TempLocation);
  > 
  > imo this should also be handled on windows via `TMP` and `TEMP`, according to
  
  Aren't those  used in the MSWin version of `TempLocation`? I can try to implement this for MSWin too but only if someone promises to test it (I can't).
  
  BTW, cf. https://codereview.qt-project.org/#/c/239952
  
  > all of this code shouldn't be required - the system env used to launch subprocesses should not use the changed tmpdir after all, so it cannot be recursive
  
  Evidently this can go once that restored system env. is used.
  
  >> +    m_tmpDir->mkpath(tmpLocation);
  > 
  > it exists, you create the path in the line above
  
  Except when mkpath failed because tmpLocation was messed up. KDevelop does NOT like that. That happened to me during development and I decided to leave the check in.
  
  >> +    void systemEnvironment(QProcessEnvironment& env) const;
  > 
  > don't use out params, just return a QProcessEnvironment similar to how it's done in `QProcessEnvironment::systemEnvironment()`
  
  This was out of a concern that certain spawned processes might be performance critical. I guess the overhead of the extra copy can be neglected then?

REPOSITORY
  R32 KDevelop

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

To: rjvbb, #kdevelop, kfunk, mwolff
Cc: aaronpuchert, mwolff, pino, kfunk, kdevelop-devel, glebaccon, hase, antismap, iodelay, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20190128/1b14e87e/attachment.html>


More information about the KDevelop-devel mailing list