Page MenuHomePhabricator

qchateau (Quentin Chateau)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 9 2020, 3:13 PM (26 w, 2 d)

Recent Activity

Apr 15 2021

Herald added a project to D97417: [clangd] use a compatible preamble for the first AST built: Restricted Project.

Have you guys been giving some thoughts to that patch ? I've been using it in my daily work since I submitted the patch, and I'd not go back

Apr 15 2021, 1:16 PM · Restricted Project, Restricted Project

Mar 4 2021

qchateau updated the diff for D97983: [clangd] strictly respect formatting range.

fix tests

Mar 4 2021, 2:49 PM · Restricted Project
qchateau requested review of D97983: [clangd] strictly respect formatting range.
Mar 4 2021, 2:29 PM · Restricted Project

Mar 3 2021

qchateau updated the diff for D97417: [clangd] use a compatible preamble for the first AST built.

fix bad arc diff

Mar 3 2021, 2:31 PM · Restricted Project, Restricted Project
qchateau updated the diff for D97417: [clangd] use a compatible preamble for the first AST built.

rebase on main, fix formatting

Mar 3 2021, 2:29 PM · Restricted Project, Restricted Project

Mar 2 2021

qchateau added a comment to D97417: [clangd] use a compatible preamble for the first AST built.

I am much more afraid of providing bad results than I am afraid of degrading performance. I mean, eventually the "real" preamble is built, and the results are just as correct as before, but it may yield incorrect results until we invalidate the AST.
That is especially the case as I'm not very experienced in compiler design and I don't have a deep understanding of what happens behind the curtains when using a PCH.
There are probably cases where we degrade perf, but overall my experience with this patch is overwhelmingly positive, especially for "long coding sessions".

Mar 2 2021, 2:25 PM · Restricted Project, Restricted Project

Feb 26 2021

qchateau updated the diff for D97417: [clangd] use a compatible preamble for the first AST built.
  • [clangd] make more compile commands compatible
  • [clangd] ignore incompatible preamble
Feb 26 2021, 3:05 PM · Restricted Project, Restricted Project

Feb 25 2021

qchateau added a comment to D97417: [clangd] use a compatible preamble for the first AST built.

The hardcoded 5 premables can indeed be changed, I did not want to waste time on coding a configuration logic at such an early stage.

Feb 25 2021, 10:04 AM · Restricted Project, Restricted Project

Feb 24 2021

qchateau requested review of D97417: [clangd] use a compatible preamble for the first AST built.
Feb 24 2021, 2:32 PM · Restricted Project, Restricted Project

Jan 25 2021

qchateau added a comment to D94875: [clangd] ignore parallelism level for quick tasks.

I applied your suggestion as I agree with both. I chose to use Opts.AsyncThreadsCount instead of a hard-coded constant. This way the "formatting speed" will grow as the user allow more parallelism, which IMO makes sense.

Jan 25 2021, 12:02 PM · Restricted Project
qchateau updated the diff for D94875: [clangd] ignore parallelism level for quick tasks.
  • [clangd] fix nits
Jan 25 2021, 11:58 AM · Restricted Project

Jan 17 2021

qchateau requested review of D94875: [clangd] ignore parallelism level for quick tasks.
Jan 17 2021, 9:59 AM · Restricted Project
qchateau added a comment to D93829: [clangd] Support outgoing calls in call hierarchy.

You're right, I probably got carried away for the sake of completeness. Allocating extra memory for a feature nobody is going to use is definitely not worth it. Though maybe we can take advantage of what's already been done and if we remove the RevRefs map, and pay the full price of iterating all refs when refersTo is called, we still have this feature. Although slower, it shouldn't take more than a few 100's of milliseconds even on a big project. Which means the feature is usable, at least for most small to medium sized projects, at no extra cost for people who do not use it.

Jan 17 2021, 9:15 AM · Restricted Project

Jan 14 2021

qchateau added a comment to D93452: [clangd] Trim memory periodically.

If you look back at my original comment, you'll notice I originally trimmed the memory after FileSymbols::buildIndex which seemed to be the "end of the peak" memory usage, the goal was to counteract exactly what you describe: a huge memory usage when clangd warms-up and parses a lot of files in parallel.
I wonder if we can find a middle ground. The solution we finally adopted also has the advantage of being less intrusive, which is definitely great considering the nature of the issue

Jan 14 2021, 11:50 AM · Restricted Project, Restricted Project

Jan 11 2021

qchateau added a comment to D93873: [clangd] Cache preambles of closed files.

Hey, don't worry about the delay, I won't have as much time on my hands anymore anyway.

Jan 11 2021, 10:20 AM · Restricted Project
qchateau added inline comments to D93829: [clangd] Support outgoing calls in call hierarchy.
Jan 11 2021, 12:25 AM · Restricted Project

Jan 9 2021

qchateau added a comment to D93873: [clangd] Cache preambles of closed files.

New year's ping ?

Jan 9 2021, 6:40 AM · Restricted Project

Jan 7 2021

qchateau abandoned D94265: [clangd] Search compiler in PATH for system include extraction If the compiler is specified by its name, search it in the system PATH instead of the command directory: this is what the build system would do..

Ah ! It's been done already !
https://reviews.llvm.org/D93600

Jan 7 2021, 2:08 PM · Restricted Project
qchateau requested review of D94265: [clangd] Search compiler in PATH for system include extraction If the compiler is specified by its name, search it in the system PATH instead of the command directory: this is what the build system would do..
Jan 7 2021, 1:56 PM · Restricted Project

Dec 30 2020

qchateau updated the diff for D93873: [clangd] Cache preambles of closed files.
  • Fix keep preamble command line option
  • Fix tests
Dec 30 2020, 3:50 PM · Restricted Project

Dec 28 2020

qchateau requested review of D93873: [clangd] Cache preambles of closed files.
Dec 28 2020, 1:51 PM · Restricted Project

Dec 27 2020

qchateau added a comment to D93829: [clangd] Support outgoing calls in call hierarchy.

Note: Build fails due to clang-tidy warnings in tests. I chose to keep the naming consistent with what's already in place rather than fix the clang-tidy warning.

Dec 27 2020, 3:55 AM · Restricted Project
qchateau updated the diff for D93829: [clangd] Support outgoing calls in call hierarchy.
  • Smaller reversed refs memory usage
  • Fix Dex::estimateMemoryUsage
Dec 27 2020, 3:53 AM · Restricted Project

Dec 26 2020

qchateau updated the diff for D93829: [clangd] Support outgoing calls in call hierarchy.
  • Fix tests
Dec 26 2020, 4:24 PM · Restricted Project
qchateau requested review of D93829: [clangd] Support outgoing calls in call hierarchy.
Dec 26 2020, 2:31 PM · Restricted Project

Dec 22 2020

qchateau accepted D93726: [clangd] Use atomics instead of locks to track periodic memory trimming.

Small nits:

  • operator() is not const but Next is mutable, seems to me you intended to have operator() as const
  • memory orders for the atomic operations can probably be downgraded to std::memory_order_acquire/std::memory_order_acq_rel. I think the first load can even be relaxed but that I'm always careful with these
Dec 22 2020, 1:12 PM · Restricted Project

Dec 21 2020

qchateau updated the diff for D93452: [clangd] Trim memory periodically.

The log is back, I renamed the CMake option and the command line option and reduced the number of preprocessor directives.
I chose not to modify the files for the gn build system, I'd rather not break it.

Dec 21 2020, 3:49 PM · Restricted Project, Restricted Project
qchateau added a comment to D93546: [clangd][NFC] Improve clangd status messages.

I guess I'll need to ask for commit access

Dec 21 2020, 10:55 AM · Restricted Project
qchateau updated the diff for D93546: [clangd][NFC] Improve clangd status messages.
  • dont capitalize first letter of status
Dec 21 2020, 3:57 AM · Restricted Project

Dec 20 2020

qchateau added a comment to D93452: [clangd] Trim memory periodically.

Here is the latest version, I believe it should mostly look like what you expect. Let me know if there is anything that you'd like me to change, it's totally fine to have a few more iterations on this diff.

Dec 20 2020, 2:44 PM · Restricted Project, Restricted Project
qchateau updated the diff for D93452: [clangd] Trim memory periodically.
  • Move platform specific code to ClangdMain
  • Generic memory cleanup callback in ClangdLSPServer
Dec 20 2020, 2:43 PM · Restricted Project, Restricted Project
qchateau added a comment to D93452: [clangd] Trim memory periodically.

Sorry to mess you around like this, but I think I might have jumped the gun in asking this to be abstracted away in Process.h.

No problem, there is not much code anyway. The thought process behind the "right" solution is by far the most important.

Dec 20 2020, 1:04 PM · Restricted Project, Restricted Project
qchateau added a comment to D93452: [clangd] Trim memory periodically.

Interestingly clangd held rock solid at 1.0GB recorded memory usage throughout.

Dec 20 2020, 4:03 AM · Restricted Project, Restricted Project

Dec 19 2020

qchateau planned changes to D93452: [clangd] Trim memory periodically.
Dec 19 2020, 3:02 AM · Restricted Project, Restricted Project

Dec 18 2020

qchateau updated the diff for D93452: [clangd] Trim memory periodically.
  • Fix clang-tidy errors
Dec 18 2020, 10:53 AM · Restricted Project, Restricted Project
qchateau added a comment to D93553: [clangd] Make our printing policies for Hover more consistent, especially tags.

Looks good to me, it removes a lot of duplication and corner cases.

Dec 18 2020, 10:32 AM · Restricted Project
qchateau requested review of D93546: [clangd][NFC] Improve clangd status messages.
Dec 18 2020, 7:59 AM · Restricted Project
qchateau added inline comments to D93227: [clangd] Smarter hover on auto and decltype.
Dec 18 2020, 7:37 AM · Restricted Project
qchateau retitled D93452: [clangd] Trim memory periodically from [clangd] Trim memory after buildINdex to [clangd] Trim memory periodically.
Dec 18 2020, 6:19 AM · Restricted Project, Restricted Project
qchateau updated the diff for D93452: [clangd] Trim memory periodically.
  • Add MallocTrim to llvm::sys::Process
  • Implement malloc trimming in ClangdLSPServer
Dec 18 2020, 6:18 AM · Restricted Project, Restricted Project
qchateau updated the diff for D93227: [clangd] Smarter hover on auto and decltype.

Fix ExpandAutoType to not expand undeduced auto

Dec 18 2020, 4:40 AM · Restricted Project
qchateau added a comment to D93227: [clangd] Smarter hover on auto and decltype.

You can land this if it is still fine after my final update.

Dec 18 2020, 3:41 AM · Restricted Project
qchateau updated the diff for D93227: [clangd] Smarter hover on auto and decltype.
  • Verify documentation in hover test on 'auto'
  • Fixed comments
  • Converted raw string to string to avoid trailing whitespace
Dec 18 2020, 3:40 AM · Restricted Project

Dec 17 2020

qchateau added inline comments to D93227: [clangd] Smarter hover on auto and decltype.
Dec 17 2020, 10:08 AM · Restricted Project
qchateau updated the diff for D93227: [clangd] Smarter hover on auto and decltype.

I updated this diff according to your review. There are a few things that may still need change but I need your input, see my other comments :)

Dec 17 2020, 10:06 AM · Restricted Project
qchateau planned changes to D93452: [clangd] Trim memory periodically.

Periodic malloc_trim seems like it solves a real problem, one I don't personally fully understand but we may never do - I feel convinced we should use it anyway. It probably has some overhead, who knows how much.

It does indeed look like facing the problem from the wrong side (fight the end result instead of finding the root cause) but at that point it does solve an annoying problem. As I investigated this, I found that replacing DenseMap and StringMap wtih std::map mitigates the issue. I think allocating huge chunks of contiguous memory temporarily make this issue worse. Not very useful, but hey, I'm just sharing what I observed. As I previously said, resolving this problem by changing containers or allocators is a lot of work and is not guaranteed to actually solve the issue.

Dec 17 2020, 8:40 AM · Restricted Project, Restricted Project
qchateau updated the diff for D93452: [clangd] Trim memory periodically.

Use malloc_trim only if available

Dec 17 2020, 7:50 AM · Restricted Project, Restricted Project
qchateau added a comment to D93452: [clangd] Trim memory periodically.

Side note is this memory failing to free behaviour observed on windows?

Dec 17 2020, 7:26 AM · Restricted Project, Restricted Project
qchateau requested review of D93452: [clangd] Trim memory periodically.
Dec 17 2020, 4:40 AM · Restricted Project, Restricted Project
qchateau added a comment to D93227: [clangd] Smarter hover on auto and decltype.

We can't easily add new SymbolKinds (it's an enum we don't own, which we should fix, but that's a yak-shave). But TypeAlias almost fits these roles for auto.

Dec 17 2020, 3:49 AM · Restricted Project
qchateau updated the diff for D93227: [clangd] Smarter hover on auto and decltype.
  • Rebase on master after D92041
  • Remove the usage of the "Documentation" field
  • Use the TypeAlias Kind on auto and decltype
  • Move code related to hover on this in a new function
  • Update hover on this to be consistent with this change
Dec 17 2020, 3:44 AM · Restricted Project

Dec 16 2020

qchateau added a comment to D93314: [clangd] go-to-definition on auto unwraps array/pointer types.

So I'm leaning towards defining these behaviors for pointers and containers rather than templates per se. There's a nice symmetry with the raw pointer and array types that this patch.
(We have some precedent and code for detecting smart pointers, see getPointeeType() in FindTarget.cpp.)

Dec 16 2020, 5:27 AM · Restricted Project

Dec 15 2020

qchateau added a comment to D93314: [clangd] go-to-definition on auto unwraps array/pointer types.

Ah good catch, I agree this is the behavior we want.

Dec 15 2020, 12:12 PM · Restricted Project

Dec 14 2020

qchateau updated the summary of D93227: [clangd] Smarter hover on auto and decltype.
Dec 14 2020, 8:41 AM · Restricted Project
qchateau requested review of D93227: [clangd] Smarter hover on auto and decltype.
Dec 14 2020, 8:36 AM · Restricted Project

Dec 11 2020

qchateau added a comment to D92977: [clangd] Improve hover and goToDefinition on auto and dectype.

I've updated the patch with your latest comments.

Dec 11 2020, 9:13 AM · Restricted Project, Restricted Project
qchateau updated the diff for D92977: [clangd] Improve hover and goToDefinition on auto and dectype.
  • Reintroduce test with a FIXME comment
  • locateSymbolForType returns a vector instead of an optional
Dec 11 2020, 9:10 AM · Restricted Project, Restricted Project
qchateau added inline comments to D92977: [clangd] Improve hover and goToDefinition on auto and dectype.
Dec 11 2020, 7:19 AM · Restricted Project, Restricted Project
qchateau updated the diff for D92977: [clangd] Improve hover and goToDefinition on auto and dectype.
  • Remove the patches affecting hover
  • Add tests in ASTTests.cpp to test the behavior of getDeducedType for auto and decltype
  • Add missing comment
  • Create a locateSymbolForType function (which uses targetDecl)
  • Remove the unit tests that were testing undesirable behaviors
Dec 11 2020, 7:04 AM · Restricted Project, Restricted Project

Dec 10 2020

qchateau added a comment to D92977: [clangd] Improve hover and goToDefinition on auto and dectype.

Not sure what's wrong with the unit test "x64 windows > LLVM.CodeGen/XCore::threads.ll", I don't see how it's related to my patch

Dec 10 2020, 8:02 AM · Restricted Project, Restricted Project
qchateau updated the diff for D92977: [clangd] Improve hover and goToDefinition on auto and dectype.

I've fixed the failing unittests, added some more
to test the new behavior, and fixed some bugs in
the new code.

Dec 10 2020, 6:09 AM · Restricted Project, Restricted Project

Dec 9 2020

qchateau updated the summary of D92977: [clangd] Improve hover and goToDefinition on auto and dectype.
Dec 9 2020, 3:37 PM · Restricted Project, Restricted Project
qchateau requested review of D92977: [clangd] Improve hover and goToDefinition on auto and dectype.
Dec 9 2020, 3:32 PM · Restricted Project, Restricted Project