RFR: 8196083: Avoid locking in OopStorage::release
Kim Barrett
kim.barrett at oracle.com
Thu Feb 8 02:24:34 UTC 2018
> On Feb 7, 2018, at 4:19 PM, coleen.phillimore at oracle.com wrote:
>
>
> Hi, I've reviewed this and I don't see anything wrong. It looks like the necessary OrderAccess and CAS instructions are used on the deferred_updates list.
Thanks.
> I think this logging should be debug mode because I don't know if you want this with -Xlog (default is log everything with info mode, I think), except this one:
>
> log_info(oopstorage, ref)("%s: failed allocation", name());
If anything, I'm tempted to make that one a warning. Odds are pretty
good that it will lead to some error or abort somewhere up the call chain.
I think the oopstorage,blocks log_info's are reasonable.
Other than the one you mention, oopstorage,ref log_info's might be a
bit much, particularly as we start making more use of oopstorage. But
I'd like to consider changes to the logging levels here as a separate
issue.
> And here, why do you need to unlink the blocks? Aren't you deleting them below from traversing the main list?
>
> OopStorage::~OopStorage() {
> Block* block;
> while ((block = _deferred_updates) != NULL) {
> _deferred_updates = block->deferred_updates_next();
> block->set_deferred_updates_next(NULL);
> }
> while ((block = _allocate_list.head()) != NULL) {
> _allocate_list.unlink(*block);
> }
>
> Can delete_block just clear all the fields in the block?
Block deletion asserts the block is no longer in use, to catch
improper deletion bugs. But if we're tearing down the whole storage
object, it doesn't matter what the current internal state is, we want
to clean it all up. So storage deletion removes blocks from lists
before deleting them.
More information about the hotspot-dev
mailing list