search

eclipse / omr

Eclipse OMR™ Cross platform components for building reliable, high performance language runtimes
http://www.eclipse.org/omr
Other
940 stars 394 forks source link

Make implementations of small SymbolReferenceTable methods inline #3285

Open hzongaro opened 5 years ago

hzongaro commented 5 years ago

In a review comment for pull request https://github.com/eclipse/omr/pull/3248#discussion_r236836454, Gita @gita-omr recommended that the SymbolReferenceTable::isNonHelpler and SymbolReferenceTable::getNonHelperSymbol methods be inlined. I ran into some difficulties, outlined later in that review thread https://github.com/eclipse/omr/pull/3248#discussion_r238776374, and so I've separated out that task into this separate issue.

hzongaro commented 5 years ago

For convenience, I'll repeat the two approaches mentioned in the comments from pull request #3248:

This could be tackled by adding #includes for the new _inlines.hpp files to the .cpp files in stages:

  1. Add empty *SymbolReferenceTable_inlines.hpp to OMR
  2. Add #includes for the new _inlines.hpp files to OpenJ9
  3. Actually add inline method implementations to OMR and #include for the new _inlines.hpp files to OMR (or the second half of this step could be done at step 1.)

If we want to avoid requiring #includes for the new _inlines.hpp files to the .cpp files, which I think is desirable, there are some entanglements between the implementations of SymbolReference and SymbolReferenceTable that will complicate things.

Leonardo2718 commented 5 years ago

Minor side note: some classes get away with including the <class_name>_inlines.hpp file right at the end of the <class_name>.hpp file. For example:

https://github.com/eclipse/omr/blob/06e06a4175f841be0386dda8a5c41a33d2bb86e5/compiler/il/Symbol.hpp#L75-L78

This avoids having to include the _inlines.hpp file in all the .cpp files that need it but it also deviates a little from the goal of keeping the <class_name>.hpp files minimal.