JDK-8234628 : Change BasicHashTables::new_entry() to use clamp()
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 14
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2019-11-22
  • Updated: 2024-10-17
  • Resolved: 2020-06-02
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 15
15 b26Fixed
Related Reports
Relates :  
JDK-8233702 - Introduce helper function to clamp value to range
Description
The changes in JDK-8233702 added a clamp() function to clamp a value between given min and max, with the assert checking whether min <= max.

Attempting to use this method in

hashtable.cpp:

template <MEMFLAGS F> BasicHashtableEntry<F>* BasicHashtable<F>::new_entry(unsigned int hashValue) {

64:       int block_size = MIN2(512, MAX2((int)_table_size / 2, (int)_number_of_entries));

caused the assert to fail which indicates a potential issue with the calculation, i.e. unexpected return values, and so unexpected size of the hash table

Please look at the method/calculation and if the clamp() method should be used, and if so use it after fixing the code. Otherwise it would be nice to add an explanation why the method is not used/why the code is as is.

Otherwise just close out this issue.
Comments
URL: https://hg.openjdk.java.net/jdk/jdk/rev/8eea00250449 User: iklam Date: 2020-06-02 16:29:43 +0000
02-06-2020
JDK-8233702 implements this: template<typename T> inline T clamp(T value, T min, T max) { assert(min <= max, "must be"); return MIN2(MAX2(value, min), max); } But the hashtable code noted in the Description does not fall into this pattern: int block_size = MIN2(512, MAX2((int)_table_size / 2, (int)_number_of_entries)); It's possible for the 3 parameters used here to be in any arbitrary order. The intention of the code is "to pick something that seems appropriate, but never exceed 512". The latest version can be expressed with the clamp() pattern to make it more readable: int block_size = MAX2((int)_table_size / 2, (int)_number_of_entries)); // pick something that seems appropriate block_size = clamp(block_size, 2, 512); // but never go out of this range
02-06-2020
ILW = MLM = P4
26-11-2019
[~tschatzl] the line you mentioned in the Description has been changed to this in JDK-8234127 http://hg.openjdk.java.net/jdk/jdk/file/438337c846fb/src/hotspot/share/utilities/hashtable.cpp#l64 - int block_size = MIN2(512, MAX2((int)_table_size / 2, (int)_number_of_entries)); + int block_size = MIN2(512, MAX3(2, (int)_table_size / 2, (int)_number_of_entries));
22-11-2019