8198369: Clean up GCId and GCIdMark

Reviewed-by: stefank, eosterlund

src/hotspot/share/gc/cms/concurrentMarkSweepGeneration.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -5551,7 +5551,7 @@ void CMSCollector::reset_stw() {
   // already have the lock
   assert(_collectorState == Resetting, "just checking");
   assert_lock_strong(bitMapLock());
-  GCIdMarkAndRestore gc_id_mark(_cmsThread->gc_id());
+  GCIdMark gc_id_mark(_cmsThread->gc_id());
   _markBitMap.clear_all();
   _collectorState = Idling;
   register_gc_end();

src/hotspot/share/gc/g1/concurrentMarkThread.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -268,8 +268,6 @@ void ConcurrentMarkThread::run_service() {
 
     cm()->concurrent_cycle_start();
 
-    assert(GCId::current() != GCId::undefined(), "GC id should have been set up by the initial mark GC.");
-
     GCTraceConcTime(Info, gc) tt("Concurrent Cycle");
     {
       ResourceMark rm;

src/hotspot/share/gc/shared/gcId.cpp

@@ -35,21 +35,21 @@ NamedThread* currentNamedthread() {
   return (NamedThread*)Thread::current();
 }
 
-const uint GCId::create() {
+uint GCId::create() {
   return _next_id++;
 }
 
-const uint GCId::peek() {
+uint GCId::peek() {
   return _next_id;
 }
 
-const uint GCId::current() {
+uint GCId::current() {
   const uint gc_id = currentNamedthread()->gc_id();
   assert(gc_id != undefined(), "Using undefined GC id.");
   return gc_id;
 }
 
-const uint GCId::current_or_undefined() {
+uint GCId::current_or_undefined() {
   return Thread::current()->is_Named_thread() ? currentNamedthread()->gc_id() : undefined();
 }
 
@@ -66,28 +66,14 @@ size_t GCId::print_prefix(char* buf, size_t len) {
   return 0;
 }
 
-GCIdMark::GCIdMark() : _gc_id(GCId::create()) {
-  currentNamedthread()->set_gc_id(_gc_id);
+GCIdMark::GCIdMark() : _previous_gc_id(currentNamedthread()->gc_id()) {
+  currentNamedthread()->set_gc_id(GCId::create());
 }
 
-GCIdMark::GCIdMark(uint gc_id) : _gc_id(gc_id) {
-  currentNamedthread()->set_gc_id(_gc_id);
+GCIdMark::GCIdMark(uint gc_id) : _previous_gc_id(currentNamedthread()->gc_id()) {
+  currentNamedthread()->set_gc_id(gc_id);
 }
 
 GCIdMark::~GCIdMark() {
-  currentNamedthread()->set_gc_id(GCId::undefined());
-}
-
-GCIdMarkAndRestore::GCIdMarkAndRestore() : _gc_id(GCId::create()) {
-  _previous_gc_id = currentNamedthread()->gc_id();
-  currentNamedthread()->set_gc_id(_gc_id);
-}
-
-GCIdMarkAndRestore::GCIdMarkAndRestore(uint gc_id) : _gc_id(gc_id) {
-  _previous_gc_id = currentNamedthread()->gc_id();
-  currentNamedthread()->set_gc_id(_gc_id);
-}
-
-GCIdMarkAndRestore::~GCIdMarkAndRestore() {
   currentNamedthread()->set_gc_id(_previous_gc_id);
 }

src/hotspot/share/gc/shared/gcId.hpp

@@ -28,38 +28,32 @@
 #include "memory/allocation.hpp"
 
 class GCId : public AllStatic {
+private:
   friend class GCIdMark;
-  friend class GCIdMarkAndRestore;
+
   static uint _next_id;
   static const uint UNDEFINED = (uint)-1;
-  static const uint create();
+  static uint create();
 
- public:
+public:
   // Returns the currently active GC id. Asserts that there is an active GC id.
-  static const uint current();
+  static uint current();
   // Same as current() but can return undefined() if no GC id is currently active
-  static const uint current_or_undefined();
+  static uint current_or_undefined();
   // Returns the next expected GCId.
-  static const uint peek();
-  static const uint undefined() { return UNDEFINED; }
+  static uint peek();
+  static uint undefined() { return UNDEFINED; }
   static size_t print_prefix(char* buf, size_t len);
 };
 
 class GCIdMark : public StackObj {
-  uint _gc_id;
- public:
+private:
+  const uint _previous_gc_id;
+
+public:
   GCIdMark();
   GCIdMark(uint gc_id);
   ~GCIdMark();
 };
 
-class GCIdMarkAndRestore : public StackObj {
-  uint _gc_id;
-  uint _previous_gc_id;
- public:
-  GCIdMarkAndRestore();
-  GCIdMarkAndRestore(uint gc_id);
-  ~GCIdMarkAndRestore();
-};
-
 #endif // SHARE_VM_GC_SHARED_GCID_HPP

src/hotspot/share/gc/shared/genCollectedHeap.cpp

@@ -367,7 +367,7 @@ void GenCollectedHeap::do_collection(bool           full,
     return; // GC is disabled (e.g. JNI GetXXXCritical operation)
   }
 
-  GCIdMarkAndRestore gc_id_mark;
+  GCIdMark gc_id_mark;
 
   const bool do_clear_all_soft_refs = clear_all_soft_refs ||
                           collector_policy()->should_clear_all_soft_refs();
@@ -442,7 +442,7 @@ void GenCollectedHeap::do_collection(bool           full,
 
       if (do_young_collection) {
         // We did a young GC. Need a new GC id for the old GC.
-        GCIdMarkAndRestore gc_id_mark;
+        GCIdMark gc_id_mark;
         GCTraceTime(Info, gc) t("Pause Full", NULL, gc_cause(), true);
         collect_generation(_old_gen, full, size, is_tlab, run_verification && VerifyGCLevel <= 1, do_clear_all_soft_refs, true);
       } else {