RFR: 8314029: Add file name parameter to Compiler.perfmap [v5]

Evgeny Astigeevich eastigeevich at openjdk.org
Tue Dec 5 16:17:41 UTC 2023


On Fri, 1 Dec 2023 21:25:19 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>> Yi-Fan Tsai has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Apply man changes
>
> src/hotspot/share/code/codeCache.cpp line 1809:
> 
>> 1807: }
>> 1808: 
>> 1809: void CodeCache::write_perf_map(const char* filename) {
> 
> Why not have a `filename == nullptr` indicate that the default should be used. Then you don't need CodeCache::DefaultPerfMapFile. You can just have a private `CodeCache::defaultPerfmapFileName()` method.

Hi Chris,
The current design of `write_perf_map` provides a clean and explicit interface. The purpose of the function is evident from its signature: to write a perf map into a specified file. This explicitness makes the code more readable and self-documenting. It reduces the need for developers to go to the implementation to figure out: what is the meaning of `nullptr`; where a filename will be taken from. It also serves as a contract between the caller and the function itself. By explicitly requiring a filename, the function sets clear expectations for the caller.

I think `CodeCache::write_default_perf_map` hiding the filename of the default perf map might not be a good idea because it makes impossible to get the filename used in it.  I prefer either method `CodeCache::defaultPerfmapFileName()` or class `CodeCache::DefaultPerfmapFileName`. The class is simpler to implement than the method (like it was earlier).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15871#discussion_r1415892122


More information about the serviceability-dev mailing list