RFR: 8271525: ResourceHashtableBase::iterate() should not declared as const

[Ioi Lam]

On Fri, 30 Jul 2021 16:10:29 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

> Hi Ioi,
> 
> > Hi Thomas,
> > I've added `const V* get(K const& key) const;` as you suggested.
> > However, I don't think `iterate()` should be const, because we have code that actually modifies the table's contents:
> 
> I disagree; iterate does not modify the table. Calling it from a const context (eg. to print the table) should be possible.

It does. The code I pointed below modifies the `counter` which is stored inside the table. It's possible to write a iterator that prints out all the counters, and resets all of them to zero. To me, this is a modification of the table. The current API does not forbid that, so it should not be declared `const`.

We need to be consistent:

- `V* get(key)` returns a `V*` that can be used to modify the contents in the table.
- `iterate(iter)` passes a `V*` that can be used to modify the contents in the table.

So we should declare both of these functions const (or declare both of them non-const).

I think non-const makes more sense. I have a hard time understand why add/removing items are considered non-const, but modifying items are considered const.

> > https://github.com/openjdk/jdk/blob/77fbd99f792c42bb92a240d38f35e3af25500f99/src/hotspot/share/logging/logAsyncWriter.cpp#L98-L107
> > 
> > I'll try to refactor the code to have a `const_iterate()` which calls `ITER::do_entry(const&K, const &V)`.
> 
> If ITER were not a template parameter but a real functor, this would make sense since we could say:
> 
> ```
> void iterate(iterator& it);
> void iterate(const_iterator& it) const;
> ```
> 
> but with ITER being a template parameter, I don't know of a way to enforce the use of only const ITERs with a const_iterate. Therefore I am not sure a const_iterate would give us that much.

My current implementation of `const_iterate()` already enforces that `ITER::do_entry()` must be declared with `(K const&, V const&)`, due to this call:


bool cont = iter->do_entry(const_cast<K const&>(node->_key), const_cast<VALUE>(node->_value));


That's because `const_iterate()` instantiates `VALUE` as `V const&`.

-------------

PR: https://git.openjdk.java.net/jdk/pull/4942