[PATCH] D110450: [LLD] Remove global state in lld/COFF

[Amy Huang via Phabricator via llvm-commits]

akhuang added inline comments.


================
Comment at: lld/COFF/DLL.cpp:107
+private:
+  COFFLinkerContext &ctx;
 };
----------------
aganea wrote:
> I'm seeing that a lot of the `*Chunk` classes have this extra context now. Have you tried to measure peak RAM usage when linking large executables? Also in scenarios when `/Gy` is used, for example MinGW seems to use that.
> 
> My point being, if we can pass the context by argument that would be better maybe instead of storing it into these tiny objects. But I realize that isn't always practical, thus the global access proposed in D108850 - `COFFLinkerContext` would simply inherit  from `CommonLinkingContext` and then could be accessed from anywhere by a call to `context()`.
Yeah, it feels non-ideal to store the reference in a bunch of Chunks, I'll try to see whether it's feasible to pass it by argument.

Memory usage doesn't seem to increase too much, though -- peak memory usage for linking chrome (measured with https://github.com/sgraham/tim/blob/master/tim.exe) goes up by 4mb (3972mb -> 3976mb). I think chrome builds with `/Gy`. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110450/new/

https://reviews.llvm.org/D110450