RFR: 8200735: Move CMS specific code from binaryTreeDictionary and freeList to CMS files (original) (raw)

Stefan Karlsson stefan.karlsson at oracle.com
Wed Apr 4 18:02:05 UTC 2018

I've updated the patch so that we maintain the order of the moved classes and functions:  http://cr.openjdk.java.net/~stefank/8200735/webrev.02 http://cr.openjdk.java.net/~stefank/8200735/webrev.02.delta

And here's the diff of the important parts of the old binaryTreeDictionary.cpp and the new compactibleFreeListSpace.cpp files:  http://cr.openjdk.java.net/~stefank/8200735/webrev.02/oldBinaryTreeDictionaryCppVSnewCompactibleFreeListSpaceCpp.patch

Thanks, StefanK

On 2018-04-04 19:13, Stefan Karlsson wrote:

On 2018-04-04 18:34, Aleksey Shipilev wrote:

On 04/04/2018 03:22 PM, Stefan Karlsson wrote:

Please review this patch to move CMS specific code out of the binaryTreeDictionary.*pp and freeList.*hpp files into CMS files.

http://cr.openjdk.java.net/~stefank/8200735/webrev.01 Very nice cleanup, looks good! Thanks, Aleksey! I assume these are only code moves, and no actual code changes hidden there. I've tried to only make changes that would cause the compiler to generate equivalent code, minus the fact that it now might inline more (which is a consequence of the removal of the explicit template instantiation). It might be important to not that I did some changes to the classes that were brought over to CMS. For example: -template <class Chunkt, class FreeListt> -class BeginSweepClosure : public AscendTreeCensusClosure<Chunkt,_ _FreeListt> { -  double percentage; -  float intersweepcurrent; -  float intersweepestimate; -  float intrasweepestimate; - - public: -  BeginSweepClosure(double p, float intersweepcurrent, -                              float intersweepestimate, -                              float intrasweepestimate) : -   percentage(p), -   intersweepcurrent(intersweepcurrent), -   intersweepestimate(intersweepestimate), -   intrasweepestimate(intrasweepestimate) { } - -  void dolist(FreeList* fl) {} - -#if INCLUDEALLGCS -  void dolist(AdaptiveFreeList* fl) { -    double coalSurplusPercent = percentage; -    fl->computedesired(intersweepcurrent, intersweepestimate, intrasweepestimate); -    fl->setcoaldesired((ssizet)((double)fl->desired() * coalSurplusPercent)); -    fl->setbeforesweep(fl->count()); -    fl->setbfrsurp(fl->surplus()); -  } -#endif // INCLUDEALLGCS -}; became: +class BeginSweepClosure : public AscendTreeCensusClosure<FreeChunk,_ _AdaptiveFreeList > { +  double percentage; +  float intersweepcurrent; +  float intersweepestimate; +  float intrasweepestimate; + + public: +  BeginSweepClosure(double p, float intersweepcurrent, +                              float intersweepestimate, +                              float intrasweepestimate) : +   percentage(p), +   intersweepcurrent(intersweepcurrent), +   intersweepestimate(intersweepestimate), +   intrasweepestimate(intrasweepestimate) { } + +  void dolist(AdaptiveFreeList* fl) { +    double coalSurplusPercent = percentage; +    fl->computedesired(intersweepcurrent, intersweepestimate, intrasweepestimate); +    fl->setcoaldesired((ssizet)((double)fl->desired() * coalSurplusPercent)); +    fl->setbeforesweep(fl->count()); +    fl->setbfrsurp(fl->surplus()); +  } +}; That is, I removed the template parameters of the class and got rid of the unused dolist overload. I think it's easiest to see this if one diffs the old binaryTreeDictionary.cpp and the new compactibleFreeListSpace.cpp. Unfortunately, I didn't maintain the order of the classes and functions when I brought them over. I should probably do that change so that this gets easier to review. Thanks, StefanK Thanks, -Aleksey

More information about the hotspot-dev mailing list