[PATCH] D31364: LTO: Reduce memory consumption by creating an in-memory symbol table for InputFiles. NFCI.

[Peter Collingbourne via llvm-commits]

On Wed, Jun 7, 2017 at 2:35 PM, Mehdi AMINI <joker.eph at gmail.com> wrote:

>
> On Tue, Jun 6, 2017 at 3:26 PM Peter Collingbourne via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
>> pcc added inline comments.
>>
>>
>> ================
>> Comment at: llvm/include/llvm/Object/IRSymtab.h:123
>> +/// Fills in Symtab and Strtab with a valid symbol and string table for
>> Mods.
>> +Error write(ArrayRef<Module *> Mods, SmallVector<char, 0> &Symtab,
>> +            SmallVector<char, 0> &Strtab);
>> ----------------
>> pcc wrote:
>> > tejohnson wrote:
>> > > The name "write" here seems unexpected to me, since we aren't writing
>> to disk e.g.. The client does a "write" which involves a Writer class,
>> followed by a Reader, when together both are needed to essentially "read"
>> the symbols from Modules. Maybe "buildSymbolTable" or something like that.
>> The Writer is more like a Builder.
>> > Renamed to "build" (likewise to "Builder").
>> Every time I go into this code it bugs me a little that we have a reader
>> and a builder (as opposed to a writer). I hate to bring up a bikeshedding
>> topic, and it's not a big deal I guess, but would you mind if I rename this
>> back to write/Writer? In support of my position I point to the many Writer
>> classes we have in LLVM [1] that are not necessarily writing to disk.
>
>
> To me a "writer" is doing some sort of serialization while a builder is
> constructing structured representation / program entities. There is not
> necessarily a disk involved.
> So conceptually are you "building a SymTab from a Module" or "Writing a
> Module into a SymTab"?
>

It's a little of both really. I guess I'd prefer to use a name which is
symmetrical with the other thing in the file.

But anyway, that's all the bikeshedding I'm doing on this topic, I guess
I'll just leave this as is if that isn't persuasive.

Thanks,
-- 
-- 
Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170607/0c7bd116/attachment.html>