RFR: 8364128: Improve gathering of cpu feature names using FormatBuffer

[Johan Sjölen]

On Mon, 28 Jul 2025 18:16:16 GMT, Ashutosh Mehra <asmehra at openjdk.org> wrote:

> This PR implements the code for cpu features names string using FormatBuffer. It also improves and extends FormatBuffer with an additional method that appends comma separated strings to the buffer. This is useful in creating the cpu features names string.
> This code will also be useful in Leyden to implement cpu feature check for AOTCodeCache [0].
> Platforms affected: x86-64 and aarch64
> Other platforms can be done if and when Leyden changes are ported to them.
> 
> [0] https://github.com/openjdk/leyden/pull/84

It seems like we can just use a `stringStream` instead and not have to deal with any overflow of some buffer, since we strdup at the end.

src/hotspot/cpu/x86/vm_version_x86.cpp line 1111:

> 1109:   assert(!info_buffer.overflow(), "not enough buffer size");
> 1110:   int features_offset = info_buffer.length();
> 1111:   insert_features_names(_features, info_buffer);

Can't you just check the overflow at the end?

src/hotspot/cpu/x86/vm_version_x86.cpp line 1113:

> 1111:   insert_features_names(_features, info_buffer);
> 1112: 
> 1113:   _cpu_info_string = os::strdup(info_buffer);

Sorry, I'm going to need some help here :-). How can passing a `FormatBuffer` to `os::strdup` not fail to compile?

src/hotspot/share/runtime/abstract_vm_version.hpp line 48:

> 46: #endif // CPU_INFO_BUF_SIZE
> 47: 
> 48: template<size_t bufsz> class FormatBuffer;

Style: `template <size_t bufsz>`.

src/hotspot/share/runtime/abstract_vm_version.hpp line 50:

> 48: template<size_t bufsz> class FormatBuffer;
> 49: 
> 50: using CpuInfoBuffer = FormatBuffer<CPU_INFO_BUF_SIZE>;

Is it important that we avoid dynamic allocation at this stage of VM initialization? Is that why we can't use `stringStream` instead?

src/hotspot/share/utilities/formatBuffer.hpp line 90:

> 88:     }
> 89:     return;
> 90:   }

This method seems overly specific to the use-case.

Something like this is more widely applicable.

```c++
// Append strings returned by gen, separating each with separator.
// Stops when gen returns null or when buffer is out of space.
template <typename Generator>
void join(Generator gen, const char* separator) {
    bool first = true;
    const char* str = gen();
    while (str != nullptr) {
      const char* sep = first ? "" : separator;
      int result = append("%s%s", sep, str);
      if (result < 0) {
        return;
      }
      first = false;
    }
  return;
}


Example usage:

```c++
void VM_Version::insert_features_names(uint64_t features, CpuInfoBuffer& info_buffer) {
  int i = 0;
  info_buffer.join([&]() {
    while (!supports_feature((VM_Version::Feature_Flag)i) && i < MAX_CPU_FEATURES) {
      i++;
    }
    if (i >= MAX_CPU_FEATURES) {
      return nullptr;
    }
    return _features_names[i];
  }, ", ");
  assert(!info_buffer.overflow(), "not enough buffer size");
}

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

PR Review: https://git.openjdk.org/jdk/pull/26515#pullrequestreview-3065756178
PR Review Comment: https://git.openjdk.org/jdk/pull/26515#discussion_r2238736430
PR Review Comment: https://git.openjdk.org/jdk/pull/26515#discussion_r2238721100
PR Review Comment: https://git.openjdk.org/jdk/pull/26515#discussion_r2238700284
PR Review Comment: https://git.openjdk.org/jdk/pull/26515#discussion_r2238707903
PR Review Comment: https://git.openjdk.org/jdk/pull/26515#discussion_r2238778395