[ntcore] Match standard handle layout, only allow 16 instances (#3577)

Now that there are only 16 instances, store them all statically.

Make tests more reliable by using different ports for each connection in listener tests.
This commit is contained in:
Peter Johnson
2021-09-17 12:11:00 -07:00
committed by GitHub
parent 263a248119
commit 161e211734
7 changed files with 44 additions and 75 deletions

View File

@@ -10,8 +10,8 @@
namespace nt {
// Handle data layout:
// Bits 30-28: Type
// Bits 27-20: Instance index
// Bits 30-24: Type
// Bits 23-20: Instance index
// Bits 19-0: Handle index (0/unused for instance handles)
class Handle {
@@ -40,15 +40,15 @@ class Handle {
m_handle = 0;
return;
}
m_handle = ((static_cast<int>(type) & 0xf) << 27) | ((inst & 0x7f) << 20) |
m_handle = ((static_cast<int>(type) & 0x7f) << 24) | ((inst & 0xf) << 20) |
(index & 0xfffff);
}
int GetIndex() const { return static_cast<int>(m_handle) & 0xfffff; }
Type GetType() const {
return static_cast<Type>((static_cast<int>(m_handle) >> 27) & 0xf);
return static_cast<Type>((static_cast<int>(m_handle) >> 24) & 0x7f);
}
int GetInst() const { return (static_cast<int>(m_handle) >> 20) & 0x7f; }
int GetInst() const { return (static_cast<int>(m_handle) >> 20) & 0xf; }
bool IsType(Type type) const { return type == GetType(); }
int GetTypedIndex(Type type) const { return IsType(type) ? GetIndex() : -1; }
int GetTypedInst(Type type) const { return IsType(type) ? GetInst() : -1; }

View File

@@ -7,8 +7,7 @@
using namespace nt;
std::atomic<int> InstanceImpl::s_default{-1};
std::atomic<InstanceImpl*> InstanceImpl::s_fast_instances[10];
wpi::UidVector<InstanceImpl*, 10> InstanceImpl::s_instances;
std::atomic<InstanceImpl*> InstanceImpl::s_instances[kNumInstances];
wpi::mutex InstanceImpl::s_mutex;
using namespace std::placeholders;
@@ -35,36 +34,10 @@ InstanceImpl* InstanceImpl::GetDefault() {
}
InstanceImpl* InstanceImpl::Get(int inst) {
if (inst < 0) {
if (inst < 0 || inst >= kNumInstances) {
return nullptr;
}
// fast path, just an atomic read
if (static_cast<unsigned int>(inst) <
(sizeof(s_fast_instances) / sizeof(s_fast_instances[0]))) {
InstanceImpl* ptr = s_fast_instances[inst];
if (ptr) {
return ptr;
}
}
// slow path
std::scoped_lock lock(s_mutex);
// static fast-path block
if (static_cast<unsigned int>(inst) <
(sizeof(s_fast_instances) / sizeof(s_fast_instances[0]))) {
// double-check
return s_fast_instances[inst];
}
// vector
if (static_cast<unsigned int>(inst) < s_instances.size()) {
return s_instances[inst];
}
// doesn't exist
return nullptr;
return s_instances[inst];
}
int InstanceImpl::GetDefaultIndex() {
@@ -94,28 +67,23 @@ int InstanceImpl::Alloc() {
}
int InstanceImpl::AllocImpl() {
unsigned int inst = s_instances.emplace_back(nullptr);
InstanceImpl* ptr = new InstanceImpl(inst);
s_instances[inst] = ptr;
if (inst < (sizeof(s_fast_instances) / sizeof(s_fast_instances[0]))) {
s_fast_instances[inst] = ptr;
int inst = 0;
for (; inst < kNumInstances; ++inst) {
if (!s_instances[inst]) {
s_instances[inst] = new InstanceImpl(inst);
return inst;
}
}
return static_cast<int>(inst);
return -1;
}
void InstanceImpl::Destroy(int inst) {
std::scoped_lock lock(s_mutex);
if (inst < 0 || static_cast<unsigned int>(inst) >= s_instances.size()) {
if (inst < 0 || inst >= kNumInstances) {
return;
}
if (static_cast<unsigned int>(inst) <
(sizeof(s_fast_instances) / sizeof(s_fast_instances[0]))) {
s_fast_instances[inst] = nullptr;
}
delete s_instances[inst];
s_instances.erase(inst);
InstanceImpl* ptr = nullptr;
s_instances[inst].exchange(ptr);
delete ptr;
}

View File

@@ -8,7 +8,6 @@
#include <atomic>
#include <memory>
#include <wpi/UidVector.h>
#include <wpi/mutex.h>
#include "ConnectionNotifier.h"
@@ -47,8 +46,8 @@ class InstanceImpl {
static int AllocImpl();
static std::atomic<int> s_default;
static std::atomic<InstanceImpl*> s_fast_instances[10];
static wpi::UidVector<InstanceImpl*, 10> s_instances;
static constexpr int kNumInstances = 16;
static std::atomic<InstanceImpl*> s_instances[kNumInstances];
static wpi::mutex s_mutex;
};

View File

@@ -41,9 +41,9 @@ class ConnectionListenerTest {
}
/** Connect to the server. */
private void connect() {
m_serverInst.startServer("connectionlistenertest.ini", "127.0.0.1", 10000);
m_clientInst.startClient("127.0.0.1", 10000);
private void connect(int port) {
m_serverInst.startServer("connectionlistenertest.ini", "127.0.0.1", port);
m_clientInst.startClient("127.0.0.1", port);
// wait for client to report it's started, then wait another 0.1 sec
try {
@@ -66,7 +66,7 @@ class ConnectionListenerTest {
assertNotSame(handle, 0, "bad listener handle");
// trigger a connect event
connect();
connect(10020);
// get the event
assertTrue(m_serverInst.waitForConnectionListenerQueue(1.0));
@@ -106,16 +106,19 @@ class ConnectionListenerTest {
assertFalse(events[0].connected);
}
private static int threadedPort = 10001;
@ParameterizedTest
@DisabledOnOs(OS.WINDOWS)
@ValueSource(strings = {"127.0.0.1", "127.0.0.1 ", " 127.0.0.1 "})
void testThreaded(String address) {
m_serverInst.startServer("connectionlistenertest.ini", address, 10000);
m_serverInst.startServer("connectionlistenertest.ini", address, threadedPort);
List<ConnectionNotification> events = new ArrayList<>();
final int handle = m_serverInst.addConnectionListener(events::add, false);
// trigger a connect event
m_clientInst.startClient(address, 10000);
m_clientInst.startClient(address, threadedPort);
threadedPort++;
// wait for client to report it's started, then wait another 0.1 sec
try {

View File

@@ -35,8 +35,8 @@ class EntryListenerTest {
}
private void connect() {
m_serverInst.startServer("connectionlistenertest.ini", "127.0.0.1", 10000);
m_clientInst.startClient("127.0.0.1", 10000);
m_serverInst.startServer("connectionlistenertest.ini", "127.0.0.1", 10010);
m_clientInst.startClient("127.0.0.1", 10010);
// Use connection listener to ensure we've connected
int poller = NetworkTablesJNI.createConnectionListenerPoller(m_clientInst.getHandle());

View File

@@ -22,17 +22,16 @@ class ConnectionListenerTest : public ::testing::Test {
nt::DestroyInstance(client_inst);
}
void Connect();
void Connect(unsigned int port);
protected:
NT_Inst server_inst;
NT_Inst client_inst;
};
void ConnectionListenerTest::Connect() {
nt::StartServer(server_inst, "connectionlistenertest.ini", "127.0.0.1",
10000);
nt::StartClient(client_inst, "127.0.0.1", 10000);
void ConnectionListenerTest::Connect(unsigned int port) {
nt::StartServer(server_inst, "connectionlistenertest.ini", "127.0.0.1", port);
nt::StartClient(client_inst, "127.0.0.1", port);
// wait for client to report it's started, then wait another 0.1 sec
while ((nt::GetNetworkMode(client_inst) & NT_NET_MODE_STARTING) != 0) {
@@ -50,7 +49,7 @@ TEST_F(ConnectionListenerTest, Polled) {
ASSERT_NE(handle, 0u);
// trigger a connect event
Connect();
Connect(10000);
// get the event
ASSERT_TRUE(nt::WaitForConnectionListenerQueue(server_inst, 1.0));
@@ -85,7 +84,7 @@ TEST_F(ConnectionListenerTest, Threaded) {
false);
// trigger a connect event
Connect();
Connect(10001);
ASSERT_TRUE(nt::WaitForConnectionListenerQueue(server_inst, 1.0));

View File

@@ -35,16 +35,16 @@ class EntryListenerTest : public ::testing::Test {
nt::DestroyInstance(client_inst);
}
void Connect();
void Connect(unsigned int port);
protected:
NT_Inst server_inst;
NT_Inst client_inst;
};
void EntryListenerTest::Connect() {
nt::StartServer(server_inst, "entrylistenertest.ini", "127.0.0.1", 10000);
nt::StartClient(client_inst, "127.0.0.1", 10000);
void EntryListenerTest::Connect(unsigned int port) {
nt::StartServer(server_inst, "entrylistenertest.ini", "127.0.0.1", port);
nt::StartClient(client_inst, "127.0.0.1", port);
// Use connection listener to ensure we've connected
NT_ConnectionListenerPoller poller =
@@ -81,7 +81,7 @@ TEST_F(EntryListenerTest, EntryNewLocal) {
}
TEST_F(EntryListenerTest, DISABLED_EntryNewRemote) {
Connect();
Connect(10010);
if (HasFatalFailure()) {
return;
}
@@ -135,7 +135,7 @@ TEST_F(EntryListenerTest, PrefixNewLocal) {
}
TEST_F(EntryListenerTest, DISABLED_PrefixNewRemote) {
Connect();
Connect(10011);
if (HasFatalFailure()) {
return;
}