[PATCH] Separate file parsing from File's constructors.

Rui Ueyama ruiu at google.com
Thu Dec 11 18:29:00 PST 2014


Hi kledzik, Bigcheese, atanasyan, shankarke, Jean-Daniel,

This is a second patch for InputGraph cleanup.

Sorry about the size of the patch, but what I did in this
patch is basically moving code from constructor to a new
method, parse(), so the amount of new code is small.
This has no change in functionality.

We've discussed the issue that we have too many classes
to represent a concept of "file". We have File subclasses
that represent files read from disk. In addition to that,
we have bunch of InputElement subclasses (that are part
of InputGraph) that represent command line arguments for
input file names. InputElement is a wrapper for File.

InputElement has parseFile method. The method instantiates
a File. The File's constructor reads a file from disk and
parses that.

Because parseFile method is called from multiple worker
threads, file parsing is processed in parallel. In other
words, one reason why we needed the wrapper classes is
because a File would start reading a file as soon as it
is instantiated.

So, the reason why we have too many classes here is at
least partly because of the design flaw of File class.
Just like threads in a good threading library, we need
to separate instantiation from "start" method, so that
we can instantiate File objects when we need them (which
should be very fast because it involves only one mmap()
and no real file IO) and use them directly instead of
the wrapper classes. Later, we call parse() on each
file in parallel to let them do actual file IO.

In this design, we can eliminate a reason to have the
wrapper classes.

In order to minimize the size of the patch, I didn't go so
far as to replace the wrapper classes with File classes.
The wrapper classes are still there.

In this patch, we call parse() immediately after
instantiating a File, so this really has no change in
functionality. Eventually the call of parse() should be
moved to Driver::link(). That'll be done in another patch.

http://reviews.llvm.org/D6633

Files:
  include/lld/Core/File.h
  include/lld/ReaderWriter/MachOLinkingContext.h
  lib/ReaderWriter/ELF/AArch64/AArch64ELFFile.h
  lib/ReaderWriter/ELF/DynamicFile.h
  lib/ReaderWriter/ELF/ELFFile.h
  lib/ReaderWriter/ELF/Hexagon/HexagonELFFile.h
  lib/ReaderWriter/ELF/Mips/MipsELFFile.h
  lib/ReaderWriter/ELF/PPC/PPCELFFile.h
  lib/ReaderWriter/ELF/X86/X86ELFFile.h
  lib/ReaderWriter/ELF/X86_64/X86_64ELFFile.h
  lib/ReaderWriter/FileArchive.cpp
  lib/ReaderWriter/MachO/File.h
  lib/ReaderWriter/MachO/MachONormalizedFile.h
  lib/ReaderWriter/MachO/MachONormalizedFileBinaryReader.cpp
  lib/ReaderWriter/MachO/MachONormalizedFileToAtoms.cpp
  lib/ReaderWriter/Native/ReaderNative.cpp
  lib/ReaderWriter/PECOFF/ReaderCOFF.cpp
  lib/ReaderWriter/PECOFF/ReaderImportHeader.cpp
  lib/ReaderWriter/Reader.cpp
  lib/ReaderWriter/YAML/ReaderWriterYAML.cpp

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D6633.17201.patch
Type: text/x-patch
Size: 52367 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141212/e3f7374a/attachment.bin>


More information about the llvm-commits mailing list