[PATCH] D89524: [NFC][SampleFDO] Move some common stuff from SampleProfileReaderExtBinary/SampleProfileWriterExtBinary to their parent classes.

[Wenlei He via Phabricator via llvm-commits]

wenlei added a comment.

Thanks for the refactoring. I'm wondering what is most flexible way for us to take advantage of extended binary format. I thought being able to mix and use different section is really flexible, but if we tie non-common section to specific new format/sub class, it may restrict the way we use this format. E.g. if `ExtFormatA` has `SecProfileSymbolList`, and `ExtFormatB` has something like `SecFuncMetadata` (which is what have for CSSPGO internally), then consider if there's a need for a profile to have both `SecProfileSymbolList` and `SecFuncMetadata` - implementing that through inheritance could be a bit cumbersome.

I'm thinking about two alternatives:

1. Follow similar model like ELF, adding or removing a section does not change the format "type". With that, we can pass in `SectionHdrLayout` as parameter to ctor of reader and writer, and use the layout to drive the reading and writing. In this case, we don't need to use inheritance and all new section and its reader/writer functions are always registered in a single class.
2. Still use inheritance to allow new combination of sections as a new sub format, but always register all new section and its reader/writer functions in the base class. The subclasses only define unique `SectionHdrLayout`. With this, the example above will not need to inherit from both `ExtFormatA` and `ExtFormatB`.

It's nice to keep the ability to use sections orthogonally, instead of in hierarchical manner. What do you think?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D89524