RFR: 8200735: Move CMS specific code from binaryTreeDictionary and freeList to CMS files

[Stefan Karlsson]

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 Chunk_t, class FreeList_t>
> -class BeginSweepClosure : public AscendTreeCensusClosure<Chunk_t, 
> FreeList_t> {
> -  double _percentage;
> -  float _inter_sweep_current;
> -  float _inter_sweep_estimate;
> -  float _intra_sweep_estimate;
> -
> - public:
> -  BeginSweepClosure(double p, float inter_sweep_current,
> -                              float inter_sweep_estimate,
> -                              float intra_sweep_estimate) :
> -   _percentage(p),
> -   _inter_sweep_current(inter_sweep_current),
> -   _inter_sweep_estimate(inter_sweep_estimate),
> -   _intra_sweep_estimate(intra_sweep_estimate) { }
> -
> -  void do_list(FreeList<Chunk_t>* fl) {}
> -
> -#if INCLUDE_ALL_GCS
> -  void do_list(AdaptiveFreeList<Chunk_t>* fl) {
> -    double coalSurplusPercent = _percentage;
> -    fl->compute_desired(_inter_sweep_current, _inter_sweep_estimate, 
> _intra_sweep_estimate);
> -    fl->set_coal_desired((ssize_t)((double)fl->desired() * 
> coalSurplusPercent));
> -    fl->set_before_sweep(fl->count());
> -    fl->set_bfr_surp(fl->surplus());
> -  }
> -#endif // INCLUDE_ALL_GCS
> -};
>
> became:
> +class BeginSweepClosure : public AscendTreeCensusClosure<FreeChunk, 
> AdaptiveFreeList<FreeChunk> > {
> +  double _percentage;
> +  float _inter_sweep_current;
> +  float _inter_sweep_estimate;
> +  float _intra_sweep_estimate;
> +
> + public:
> +  BeginSweepClosure(double p, float inter_sweep_current,
> +                              float inter_sweep_estimate,
> +                              float intra_sweep_estimate) :
> +   _percentage(p),
> +   _inter_sweep_current(inter_sweep_current),
> +   _inter_sweep_estimate(inter_sweep_estimate),
> +   _intra_sweep_estimate(intra_sweep_estimate) { }
> +
> +  void do_list(AdaptiveFreeList<FreeChunk>* fl) {
> +    double coalSurplusPercent = _percentage;
> +    fl->compute_desired(_inter_sweep_current, _inter_sweep_estimate, 
> _intra_sweep_estimate);
> +    fl->set_coal_desired((ssize_t)((double)fl->desired() * 
> coalSurplusPercent));
> +    fl->set_before_sweep(fl->count());
> +    fl->set_bfr_surp(fl->surplus());
> +  }
> +};
>
> That is, I removed the template parameters of the class and got rid of 
> the unused do_list 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
>
>