On Mon, 3 Apr 2023 12:59:12 GMT, Markus Grönlund <mgron...@openjdk.org> wrote:
>> src/hotspot/share/prims/agentList.cpp line 204: >> >>> 202: >>> 203: // Invokes Agent_OnAttach for agents loaded dynamically during runtime. >>> 204: jint AgentList::load_agent(const char* agent_name, const char* >>> absParam, >> >> I feel that it is better to keep the original function name >> "load_agent_library". As you listed there two kinds of agents: Java and >> Native. The function name give a hint it is native agent. Also, it is better >> to avoid changes that aren't really necessary. > > I changed the names because I found it very hard to understand what the old > names represented: "AgentLibrary" vs "Library"? "add_init_agent" vs > "add_instrumentation_agent", or even "add_loaded_agent"? Also a bit confusing > that "load_agent_library" would also include statically linked agents - no > library is loaded there. Okay. Refactoring is usually not easy to review. With a renaming it becomes harder, so it is better to be conservative. There are other side effects to consider: - back porting also becomes harder - developers have to learn new names instead of already known The good side is that your refactoring consolidates this code in a well known locations. :-) ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/12923#discussion_r1158094815