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

Stefan Karlsson stefan.karlsson at oracle.com
Wed Apr 4 17:13:27 UTC 2018


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




More information about the hotspot-dev mailing list