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

Evgeny Astigeevich eastigeevich at openjdk.org
Thu Dec 7 19:57:21 UTC 2023


On Tue, 5 Dec 2023 20:13:11 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>> 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).
>
> The default filename was already "hidden" before these changes, so at the very least things are not being made any worse, but I don't see why any users `write_perf_map` would ever need the default filename.  I just felt that adding and exporting a class whose only purpose is to provide the default name seemed like unnecessary overkill. I'm not so sure having a public CodeCache::defaultPerfmapFileName() API and two `write_perf_map` APIs isn't overkill also. There is nothing wrong with a null filename argument signally to use some default name. You can also have the filename arg default to `nullptr`.

Ok, let's have:

void CodeCache::write_perf_map(const char* filename = nullptr);


without any additional classes or funcitons.

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

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


More information about the hotspot-dev mailing list