On Fri, 30 Jul 2021 13:03:32 GMT, Zhengyu Gu <z...@openjdk.org> wrote:
> Sorry for late review. > > Did a quick scan and have a few questions, will do more detail reading later. Thanks a lot, I appreciate your feedback! > src/hotspot/share/services/nmtPreInit.hpp line 108: > >> 106: // - lookup speed is paramount since lookup is done for every >> os::free() call. >> 107: // - insert/delete speed only matters for VM startup - after NMT >> initialization the lookup >> 108: // table is readonly > > This comment does not seem to be true, you have find_and_remove() that alters > table. The point is *after NMT initialization* - `find_and_remove` only gets called before NMT initialization; after that, we only do non-modifying lookup. You'll find the logic in `NMTPreInit::handle_realloc()` and `NMTPreInit::handle_free()`, respectively. The basic idea behind this is that we remove pointers from the map as long as we can without risking concurrency issues, which is until NMT initialization. After that, we leave the map alone. It was either that or protect the map with a lock, and this is the lesser of two evils since the map is usually sparsely populated. > src/hotspot/share/services/nmtPreInit.hpp line 309: > >> 307: ::memcpy(p_new, a->payload(), MIN2(a->size, new_size)); >> 308: (*rc) = p_new; >> 309: _num_reallocs_pre_to_post++; > > post-NMT-init counter updates are racy. True, this is racy. It's just for diagnostics though - I rather remove them than make them atomic since we would pay for this with every malloc. Or, maybe atomic + debug only? ------------- PR: https://git.openjdk.java.net/jdk/pull/4874
