On Tue, 10 Nov 2020 14:15:02 GMT, Coleen Phillimore <cole...@openjdk.org> wrote:
>> This kind of use of ThreadBlockInVM predates this changeset so while >> the location is new, then code style is old, very old... I'll hold off >> changing >> this for now. > > I'd rather see ThreadBlockInVM as the convention of allowing the thread to > block if a safepoint is requested. The calls like process_if_requested are > becoming alphabet soup and keep changing, so having TBIVM is better in my > opinion. > That said, this is a strange usage. This code appears three times. It > should be a function like allow_safepoint_block(LogStream* ls, timer), with > some comment above. Then it's clear that it's checking for a safepoint in a > loop that could take a long time and the logging is ancillary. I previously looked at refactoring the three locations where `ThreadBlockInVM` is used. The problem with the refactoring is that the log messages and the parameters have some differences and some commonalities. Each of these logging sites is trying to communicate some local context that is unique to that call site along with some global context that might have changed from call site to call site. I'll take another look at refactoring shortly and will let you know what I come up with. ------------- PR: https://git.openjdk.java.net/jdk/pull/642
