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

ResourceHashtableBase::iterate() should not declared as const

    XMLWordPrintable

    Details

    • Type: Enhancement
    • Status: Resolved
    • Priority: P4
    • Resolution: Won't Fix
    • Affects Version/s: 18
    • Fix Version/s: 18
    • Component/s: hotspot
    • Labels:

      Description

      https://github.com/openjdk/jdk/blob/77fbd99f792c42bb92a240d38f35e3af25500f99/src/hotspot/share/utilities/resourceHash.hpp#L196

      The iterate function is declared `const`, which is supposed to indicate that the iteration operation must not modify the table.

        // ITER contains bool do_entry(K const&, V const&), which will be
        // called for each entry in the table. If do_entry() returns false,
        // the iteration is cancelled.
        template<class ITER>
        void iterate(ITER* iter) const {
          Node* const* bucket = table();
          const unsigned sz = table_size();
          while (bucket < bucket_at(sz)) {
            Node* node = *bucket;
            while (node != NULL) {
              bool cont = iter->do_entry(node->_key, node->_value);
              if (!cont) { return; }
              node = node->_next;
            }
            ++bucket;
          }
        }

      However, even the comment itself is wrong. The two parameters passed to iter_do_entry() are NOT of the types (K const&, V const&). Instead, they are of the types (K&, V&).

      We have some iterators that actually (and legally) modify the value:

      https://github.com/openjdk/jdk/blob/77fbd99f792c42bb92a240d38f35e3af25500f99/src/hotspot/share/logging/logAsyncWriter.cpp#L98

        bool do_entry(LogFileOutput* output, uint32_t& counter) {
          if (counter > 0) {
            ....
           _logs.push_back(msg);
            counter = 0;
          }

      Since iterate() is declared const, we are forced to declare table() and bucket_at() as const, which make it difficult to implement new non-const functions that call these two functions (see JDK-8271506).

      ======================
      Proposal:

        // bool ITER::do_entry(K const& key, V& value) will be
        // called for each entry in the table. If do_entry() returns false,
        // the iteration is cancelled.
        //
        // do_entry() may modify the value. However, it should not modify the key,
        // or else the table may no longer be properly hashed. template<class ITER>
        void iterate(ITER* iter) {
          Node ** bucket = table();
          const unsigned sz = table_size();
          while (bucket < bucket_at(sz)) {
            Node * node = *bucket;
            while (node != NULL) {
              bool cont = iter->do_entry(const_cast<K const&>(node->_key), node->_value);
              if (!cont) { return; }
              node = node->_next;
            }
            ++bucket;
          }
        }

      With this change, bucket_at() and table() can also be non-const.

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              iklam Ioi Lam
              Reporter:
              iklam Ioi Lam
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: