Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8077403

Remove guarantee from GenCollectedHeap::is_in()

XMLWordPrintable

    • Icon: Enhancement Enhancement
    • Resolution: Fixed
    • Icon: P3 P3
    • 9
    • 9
    • hotspot
    • None
    • gc
    • b64

        The method CollectedHeap::is_in() has the following comment:

          // Returns "TRUE" iff "p" points into the committed areas of the heap.
          // Since this method can be expensive in general, we restrict its
          // use to assertion checking only.

        This is incorrect since we don't just use it for asserts. We also use it in os::print_location() which is in turn is used by several different code paths, such as verification, crash dumping and logging.

        In GenCollectedHeap::is_in() there is an attempt to verify that we only call this method from places that are not performance critical. So, its implementation looks like this:

        bool GenCollectedHeap::is_in(const void* p) const {
          #ifndef ASSERT
          guarantee(VerifyBeforeGC ||
                    VerifyDuringGC ||
                    VerifyBeforeExit ||
                    VerifyDuringStartup ||
                    PrintAssembly ||
                    tty->count() != 0 || // already printing
                    VerifyAfterGC ||
            VMError::fatal_error_in_progress(), "too expensive");

          #endif
          return _young_gen->is_in(p) || _old_gen->is_in(p);
        }

        It is quite odd that we do this check here and even more strange that it is only done in GenCollectedHeap and not in the other implementations (G1CollectedHeap and ParallelScavengeHeap).

        I doubt that this check is useful. The method is actually not that slow. It basically just iterates over the three spaces in young gen to do range checks and then does a single range check on the old gen. So, 4 range checks all together.

        Rather than having GenCollectedHeap::is_in() know about all callers of it I think we should remove the guarantee.

              brutisso Bengt Rutisson (Inactive)
              brutisso Bengt Rutisson (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Created:
                Updated:
                Resolved: