On Tue, 16 Apr 2024 18:54:40 GMT, Vladimir Kozlov <k...@openjdk.org> wrote:

>> src/hotspot/share/code/codeBlob.cpp line 106:
>> 
>>> 104: 
>>> 105: // Simple CodeBlob used for simple BufferBlob.
>>> 106: CodeBlob::CodeBlob(const char* name, CodeBlobKind kind, int size, 
>>> uint16_t header_size) :
>> 
>> Just a drive-by comment.  You might be able to use delegating constructors 
>> for CodeBlob so you don't have to have the field initializations twice.  
>> Maybe the same for nmethod ?
>
> Thank you, @coleenp, foe looking on these changes.
> 
> Which fields are initialized twice? Only `_oop_maps` is set to `nullptr` 
> before we proper build oop maps in first constructor.
> 
> The only saving could be lines of code but then I would have to check that 
> `cb != nullptr` and do other additional checks which I don't think will save 
> much lines.
> 
> Separation of `nmethod` constructor for native wrappers is helping clear see 
> the difference and I would like to keep them separate. We have 
> `init_defaults()` method for similar code and I can move more code into it 
> from both constructors.

Delegating constructors are the answer to having some common 'init' functions.  
It would simply save lines of code that look the same in both constructor 
initializer lists.  But it's a drive-by comment.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18768#discussion_r1567823393

Reply via email to