Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion src/hotspot/share/classfile/symbolTable.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -56,6 +56,31 @@ const size_t ON_STACK_BUFFER_LENGTH = 128;

// --------------------------------------------------------------------------

Symbol* volatile TempSymbolCleanupDelayer::_queue[QueueSize] = {};
volatile uint TempSymbolCleanupDelayer::_index = 0;

// Keep this symbol alive for some time to allow for reuse.
// Temp symbols for the same string can often be created in quick succession,
// and this queue allows them to be reused instead of churning.
void TempSymbolCleanupDelayer::delay_cleanup(Symbol* sym) {
assert(sym != nullptr, "precondition");
sym->increment_refcount();
uint i = Atomic::add(&_index, 1u) % QueueSize;
Symbol* old = Atomic::xchg(&_queue[i], sym);
if (old != nullptr) {
old->decrement_refcount();
}
}

void TempSymbolCleanupDelayer::drain_queue() {
for (uint i = 0; i < QueueSize; i++) {
Symbol* sym = Atomic::xchg(&_queue[i], (Symbol*) nullptr);
if (sym != nullptr) {
sym->decrement_refcount();
}
}
}

inline bool symbol_equals_compact_hashtable_entry(Symbol* value, const char* key, int len) {
if (value->equals(key, len)) {
return true;
Expand Down
21 changes: 19 additions & 2 deletions src/hotspot/share/classfile/symbolTable.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -33,6 +33,16 @@
class JavaThread;
template <typename T> class GrowableArray;

class TempSymbolCleanupDelayer : AllStatic {
static Symbol* volatile _queue[];
static volatile uint _index;

public:
static const uint QueueSize = 128;
static void delay_cleanup(Symbol* s);
static void drain_queue();
};

// TempNewSymbol acts as a handle class in a handle/body idiom and is
// responsible for proper resource management of the body (which is a Symbol*).
// The body is resource managed by a reference counting scheme.
Expand All @@ -54,7 +64,14 @@ class TempNewSymbol : public StackObj {

// Conversion from a Symbol* to a TempNewSymbol.
// Does not increment the current reference count.
TempNewSymbol(Symbol *s) : _temp(s) {}
TempNewSymbol(Symbol *s) : _temp(s) {
// Delay cleanup for temp symbols. Refcount is incremented while in
// queue. But don't requeue existing entries, or entries that are held
// elsewhere - it's a waste of effort.
if (s != nullptr && s->refcount() == 1) {
TempSymbolCleanupDelayer::delay_cleanup(s);
}
}

// Copy constructor increments reference count.
TempNewSymbol(const TempNewSymbol& rhs) : _temp(rhs._temp) {
Expand Down
67 changes: 61 additions & 6 deletions test/hotspot/gtest/classfile/test_symbolTable.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -27,6 +27,14 @@
#include "threadHelper.inline.hpp"
#include "unittest.hpp"

// Helper to avoid interference from the cleanup delay queue by draining it
// immediately after creation.
TempNewSymbol stable_temp_symbol(Symbol* sym) {
TempNewSymbol t = sym;
TempSymbolCleanupDelayer::drain_queue();
return t;
}

TEST_VM(SymbolTable, temp_new_symbol) {
// Assert messages assume these symbols are unique, and the refcounts start at
// one, but code does not rely on this.
Expand All @@ -36,7 +44,7 @@ TEST_VM(SymbolTable, temp_new_symbol) {

Symbol* abc = SymbolTable::new_symbol("abc");
int abccount = abc->refcount();
TempNewSymbol ss = abc;
TempNewSymbol ss = stable_temp_symbol(abc);
ASSERT_EQ(ss->refcount(), abccount) << "only one abc";
ASSERT_EQ(ss->refcount(), abc->refcount()) << "should match TempNewSymbol";

Expand All @@ -45,8 +53,8 @@ TEST_VM(SymbolTable, temp_new_symbol) {
int efgcount = efg->refcount();
int hijcount = hij->refcount();

TempNewSymbol s1 = efg;
TempNewSymbol s2 = hij;
TempNewSymbol s1 = stable_temp_symbol(efg);
TempNewSymbol s2 = stable_temp_symbol(hij);
ASSERT_EQ(s1->refcount(), efgcount) << "one efg";
ASSERT_EQ(s2->refcount(), hijcount) << "one hij";

Expand All @@ -65,13 +73,13 @@ TEST_VM(SymbolTable, temp_new_symbol) {
TempNewSymbol s3;
Symbol* klm = SymbolTable::new_symbol("klm");
int klmcount = klm->refcount();
s3 = klm; // assignment
s3 = stable_temp_symbol(klm); // assignment
ASSERT_EQ(s3->refcount(), klmcount) << "only one klm now";

Symbol* xyz = SymbolTable::new_symbol("xyz");
int xyzcount = xyz->refcount();
{ // inner scope
TempNewSymbol s_inner = xyz;
TempNewSymbol s_inner = stable_temp_symbol(xyz);
}
ASSERT_EQ(xyz->refcount(), xyzcount - 1)
<< "Should have been decremented by dtor in inner scope";
Expand Down Expand Up @@ -166,3 +174,50 @@ TEST_VM(SymbolTable, test_cleanup_leak) {

ASSERT_EQ(entry2->refcount(), 1) << "Symbol refcount just created is 1";
}

TEST_VM(SymbolTable, test_cleanup_delay) {
// Check that new temp symbols have an extra refcount increment, which is then
// decremented when the queue spills over.

TempNewSymbol s1 = SymbolTable::new_symbol("temp-s1");
ASSERT_EQ(s1->refcount(), 2) << "TempNewSymbol refcount just created is 2";

// Fill up the queue
constexpr int symbol_name_length = 30;
char symbol_name[symbol_name_length];
for (uint i = 1; i < TempSymbolCleanupDelayer::QueueSize; i++) {
os::snprintf(symbol_name, symbol_name_length, "temp-filler-%d", i);
TempNewSymbol s = SymbolTable::new_symbol(symbol_name);
ASSERT_EQ(s->refcount(), 2) << "TempNewSymbol refcount just created is 2";
}

// Add one more
TempNewSymbol spillover = SymbolTable::new_symbol("temp-spillover");
ASSERT_EQ(spillover->refcount(), 2) << "TempNewSymbol refcount just created is 2";

// The first symbol should have been removed from the queue and decremented
ASSERT_EQ(s1->refcount(), 1) << "TempNewSymbol off queue refcount is 1";
}

TEST_VM(SymbolTable, test_cleanup_delay_drain) {
// Fill up the queue
constexpr int symbol_name_length = 30;
char symbol_name[symbol_name_length];
TempNewSymbol symbols[TempSymbolCleanupDelayer::QueueSize] = {};
for (uint i = 0; i < TempSymbolCleanupDelayer::QueueSize; i++) {
os::snprintf(symbol_name, symbol_name_length, "temp-%d", i);
TempNewSymbol s = SymbolTable::new_symbol(symbol_name);
symbols[i] = s;
}

// While in the queue refcounts are incremented
for (uint i = 0; i < TempSymbolCleanupDelayer::QueueSize; i++) {
ASSERT_EQ(symbols[i]->refcount(), 2) << "TempNewSymbol refcount in queue is 2";
}

// Draining the queue should decrement the refcounts
TempSymbolCleanupDelayer::drain_queue();
for (uint i = 0; i < TempSymbolCleanupDelayer::QueueSize; i++) {
ASSERT_EQ(symbols[i]->refcount(), 1) << "TempNewSymbol refcount after drain is 1";
}
}