mirror of
https://github.com/wpilibsuite/allwpilib
synced 2026-07-03 03:01:44 +00:00
[ntcore] Add PubSubOption to disable handle signaling (#8972)
This helps reduce lock contention on Synchronization Signal objects if the user does not intend to use this signaling path. Use this option on the RobotBase MultiSubscriber to all topics.
This commit is contained in:
@@ -95,6 +95,14 @@ public sealed interface PubSubOption {
|
||||
*/
|
||||
record Hidden(boolean enabled) implements PubSubOption {}
|
||||
|
||||
/**
|
||||
* For subscriptions, specify whether value updates should not signal the local subscription
|
||||
* handle. This option defaults to disabled.
|
||||
*
|
||||
* @param disabled True to disable, false to enable
|
||||
*/
|
||||
record DisableSignal(boolean disabled) implements PubSubOption {}
|
||||
|
||||
/** Indicates only value changes will be sent over the network (default). */
|
||||
PubSubOption SEND_CHANGES = new SendAll(false);
|
||||
|
||||
@@ -139,6 +147,12 @@ public sealed interface PubSubOption {
|
||||
/** For subscriptions, indicates the subscription is hidden from the network. */
|
||||
PubSubOption HIDDEN = new Hidden(true);
|
||||
|
||||
/** For subscriptions, indicates value updates will signal the local handle (default). */
|
||||
PubSubOption ENABLE_SIGNAL = new DisableSignal(false);
|
||||
|
||||
/** For subscriptions, indicates value updates will not signal the local handle. */
|
||||
PubSubOption DISABLE_SIGNAL = new DisableSignal(true);
|
||||
|
||||
/**
|
||||
* How frequently changes will be sent over the network. NetworkTables may send more frequently
|
||||
* than this (e.g. use a combined minimum period for all values) or apply a restricted range to
|
||||
|
||||
@@ -26,6 +26,7 @@ public class PubSubOptions {
|
||||
case PubSubOption.ExcludePublisherHandle e -> excludePublisher = e.publisher();
|
||||
case PubSubOption.ExcludeSelf s -> excludeSelf = s.enabled();
|
||||
case PubSubOption.Hidden h -> hidden = h.enabled();
|
||||
case PubSubOption.DisableSignal s -> disableSignal = s.disabled();
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -41,7 +42,8 @@ public class PubSubOptions {
|
||||
boolean disableRemote,
|
||||
boolean disableLocal,
|
||||
boolean excludeSelf,
|
||||
boolean hidden) {
|
||||
boolean hidden,
|
||||
boolean disableSignal) {
|
||||
this.pollStorage = pollStorage;
|
||||
this.periodic = periodic;
|
||||
this.excludePublisher = excludePublisher;
|
||||
@@ -53,6 +55,7 @@ public class PubSubOptions {
|
||||
this.disableLocal = disableLocal;
|
||||
this.excludeSelf = excludeSelf;
|
||||
this.hidden = hidden;
|
||||
this.disableSignal = disableSignal;
|
||||
}
|
||||
|
||||
/** Default value of periodic. */
|
||||
@@ -114,4 +117,7 @@ public class PubSubOptions {
|
||||
* this one, and the subscription will not appear in metatopics.
|
||||
*/
|
||||
public boolean hidden;
|
||||
|
||||
/** For subscriptions, don't signal the local handle when value updates are queued. */
|
||||
public boolean disableSignal;
|
||||
}
|
||||
|
||||
@@ -140,6 +140,7 @@ static wpi::nt::PubSubOptions FromJavaPubSubOptions(JNIEnv* env,
|
||||
FIELD(disableLocal, "Z");
|
||||
FIELD(excludeSelf, "Z");
|
||||
FIELD(hidden, "Z");
|
||||
FIELD(disableSignal, "Z");
|
||||
|
||||
#undef FIELD
|
||||
|
||||
@@ -156,7 +157,8 @@ static wpi::nt::PubSubOptions FromJavaPubSubOptions(JNIEnv* env,
|
||||
FIELD(bool, Boolean, disableRemote),
|
||||
FIELD(bool, Boolean, disableLocal),
|
||||
FIELD(bool, Boolean, excludeSelf),
|
||||
FIELD(bool, Boolean, hidden)};
|
||||
FIELD(bool, Boolean, hidden),
|
||||
FIELD(bool, Boolean, disableSignal)};
|
||||
|
||||
#undef GET
|
||||
#undef FIELD
|
||||
|
||||
@@ -846,7 +846,9 @@ void StorageImpl::NotifyValue(LocalTopic* topic, const Value& value,
|
||||
(!publisher || (publisher && (subscriber->config.excludePublisher !=
|
||||
publisher->handle)))) {
|
||||
subscriber->pollStorage.emplace_back(value);
|
||||
subscriber->handle.Set();
|
||||
if (!subscriber->config.disableSignal) {
|
||||
subscriber->handle.Set();
|
||||
}
|
||||
if (!subscriber->valueListeners.empty()) {
|
||||
m_listenerStorage.Notify(subscriber->valueListeners, eventFlags,
|
||||
topic->handle, 0, value);
|
||||
@@ -856,7 +858,9 @@ void StorageImpl::NotifyValue(LocalTopic* topic, const Value& value,
|
||||
|
||||
for (auto&& subscriber : topic->multiSubscribers) {
|
||||
if (subscriber->options.keepDuplicates || !isDuplicate) {
|
||||
subscriber->handle.Set();
|
||||
if (!subscriber->options.disableSignal) {
|
||||
subscriber->handle.Set();
|
||||
}
|
||||
if (!subscriber->valueListeners.empty()) {
|
||||
m_listenerStorage.Notify(subscriber->valueListeners, eventFlags,
|
||||
topic->handle, 0, value);
|
||||
@@ -1011,10 +1015,14 @@ LocalSubscriber* StorageImpl::AddLocalSubscriber(LocalTopic* topic,
|
||||
if (subscriber->active) {
|
||||
if (!topic->lastValueFromNetwork && !config.disableLocal) {
|
||||
subscriber->pollStorage.emplace_back(topic->lastValue);
|
||||
subscriber->handle.Set();
|
||||
if (!subscriber->config.disableSignal) {
|
||||
subscriber->handle.Set();
|
||||
}
|
||||
} else if (topic->lastValueFromNetwork && !config.disableRemote) {
|
||||
subscriber->pollStorage.emplace_back(topic->lastValueNetwork);
|
||||
subscriber->handle.Set();
|
||||
if (!subscriber->config.disableSignal) {
|
||||
subscriber->handle.Set();
|
||||
}
|
||||
}
|
||||
}
|
||||
return subscriber;
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
#include <stdint.h>
|
||||
|
||||
#include <cassert>
|
||||
#include <cstddef>
|
||||
#include <cstdlib>
|
||||
#include <cstring>
|
||||
#include <string_view>
|
||||
@@ -116,6 +117,9 @@ static void DisposeEvent(NT_Event* event) {
|
||||
|
||||
static PubSubOptions ConvertToCpp(const NT_PubSubOptions* in) {
|
||||
PubSubOptions out;
|
||||
if (!in) {
|
||||
return out;
|
||||
}
|
||||
out.pollStorage = in->pollStorage;
|
||||
out.periodic = in->periodic;
|
||||
out.excludePublisher = in->excludePublisher;
|
||||
@@ -127,6 +131,10 @@ static PubSubOptions ConvertToCpp(const NT_PubSubOptions* in) {
|
||||
out.disableLocal = in->disableLocal;
|
||||
out.excludeSelf = in->excludeSelf;
|
||||
out.hidden = in->hidden;
|
||||
if (in->structSize >=
|
||||
offsetof(NT_PubSubOptions, disableSignal) + sizeof(in->disableSignal)) {
|
||||
out.disableSignal = in->disableSignal;
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
|
||||
@@ -358,6 +358,12 @@ struct NT_PubSubOptions {
|
||||
* will not appear in metatopics.
|
||||
*/
|
||||
NT_Bool hidden;
|
||||
|
||||
/**
|
||||
* For subscriptions, don't signal the local handle when value updates are
|
||||
* queued.
|
||||
*/
|
||||
NT_Bool disableSignal;
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
@@ -380,6 +380,12 @@ struct PubSubOptions {
|
||||
* will not appear in metatopics.
|
||||
*/
|
||||
bool hidden = false;
|
||||
|
||||
/**
|
||||
* For subscriptions, don't signal the local handle when value updates are
|
||||
* queued.
|
||||
*/
|
||||
bool disableSignal = false;
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
@@ -208,6 +208,7 @@ classes:
|
||||
disableLocal:
|
||||
excludeSelf:
|
||||
hidden:
|
||||
disableSignal:
|
||||
inline_code: |
|
||||
// autogenerated by gen-pubsub.py
|
||||
.def(py::init([](
|
||||
@@ -221,7 +222,8 @@ classes:
|
||||
bool disableRemote,
|
||||
bool disableLocal,
|
||||
bool excludeSelf,
|
||||
bool hidden
|
||||
bool hidden,
|
||||
bool disableSignal
|
||||
) -> wpi::nt::PubSubOptions {
|
||||
return wpi::nt::PubSubOptions{
|
||||
.pollStorage = pollStorage,
|
||||
@@ -234,7 +236,8 @@ classes:
|
||||
.disableRemote = disableRemote,
|
||||
.disableLocal = disableLocal,
|
||||
.excludeSelf = excludeSelf,
|
||||
.hidden = hidden
|
||||
.hidden = hidden,
|
||||
.disableSignal = disableSignal
|
||||
};
|
||||
}),
|
||||
py::kw_only(),
|
||||
@@ -249,6 +252,7 @@ classes:
|
||||
py::arg("disableLocal") = false,
|
||||
py::arg("excludeSelf") = false,
|
||||
py::arg("hidden") = false,
|
||||
py::arg("disableSignal") = false,
|
||||
R"(
|
||||
:param pollStorage: Polling storage size for a subscription. Specifies the maximum number of
|
||||
updates NetworkTables should store between calls to the subscriber's
|
||||
@@ -276,6 +280,8 @@ classes:
|
||||
network. Note this means updates will not be received from the network
|
||||
unless another subscription overlaps with this one, and the subscription
|
||||
will not appear in metatopics.
|
||||
:param disableSignal: For subscriptions, don't signal the local handle when value updates are
|
||||
queued.
|
||||
)"
|
||||
)
|
||||
wpi::nt::meta::SubscriberOptions:
|
||||
|
||||
@@ -20,6 +20,7 @@
|
||||
#include "wpi/nt/ntcore_c.h"
|
||||
#include "wpi/nt/ntcore_cpp.hpp"
|
||||
#include "wpi/util/SpanMatcher.hpp"
|
||||
#include "wpi/util/Synchronization.hpp"
|
||||
|
||||
using ::testing::_;
|
||||
using ::testing::AllOf;
|
||||
@@ -38,7 +39,9 @@ namespace wpi::nt {
|
||||
Field("pollStorage", &PubSubOptionsImpl::pollStorage, good.pollStorage),
|
||||
Field("sendAll", &PubSubOptionsImpl::sendAll, good.sendAll),
|
||||
Field("keepDuplicates", &PubSubOptionsImpl::keepDuplicates,
|
||||
good.keepDuplicates));
|
||||
good.keepDuplicates),
|
||||
Field("disableSignal", &PubSubOptionsImpl::disableSignal,
|
||||
good.disableSignal));
|
||||
}
|
||||
|
||||
::testing::Matcher<const PubSubOptionsImpl&> IsDefaultPubSubOptions() {
|
||||
@@ -545,6 +548,55 @@ TEST_F(LocalStorageTest, SetValueInvalidHandle) {
|
||||
EXPECT_FALSE(storage.SetEntryValue(0, {}));
|
||||
}
|
||||
|
||||
TEST_F(LocalStorageTest, DisableSignalSubscriberQueuesWithoutSignaling) {
|
||||
PubSubOptionsImpl subOptions;
|
||||
subOptions.disableSignal = true;
|
||||
EXPECT_CALL(network,
|
||||
ClientSubscribe(_, wpi::util::SpanEq({std::string{"foo"}}),
|
||||
IsPubSubOptions(subOptions)));
|
||||
auto sub =
|
||||
storage.Subscribe(fooTopic, NT_DOUBLE, "double", {.disableSignal = true});
|
||||
|
||||
EXPECT_CALL(
|
||||
network,
|
||||
ClientPublish(_, std::string_view{"foo"}, std::string_view{"double"},
|
||||
wpi::util::json::object(), IsDefaultPubSubOptions()));
|
||||
auto pub = storage.Publish(fooTopic, NT_DOUBLE, "double", {}, {});
|
||||
|
||||
auto val = Value::MakeDouble(1.0, 50);
|
||||
EXPECT_CALL(network, ClientSetValue(Handle{pub}.GetIndex(), val));
|
||||
storage.SetEntryValue(pub, val);
|
||||
|
||||
bool timedOut = false;
|
||||
EXPECT_FALSE(wpi::util::WaitForObject(sub, 0, &timedOut));
|
||||
EXPECT_TRUE(timedOut);
|
||||
auto values = storage.ReadQueue<double>(sub);
|
||||
ASSERT_EQ(values.size(), 1u);
|
||||
EXPECT_EQ(values[0].value, 1.0);
|
||||
EXPECT_EQ(values[0].time, 50);
|
||||
}
|
||||
|
||||
TEST_F(LocalStorageTest, DisableSignalMultiSubscriberDoesNotSignalHandle) {
|
||||
PubSubOptionsImpl subOptions;
|
||||
subOptions.disableSignal = true;
|
||||
EXPECT_CALL(network, ClientSubscribe(_, _, IsPubSubOptions(subOptions)));
|
||||
auto sub = storage.SubscribeMultiple({{""}}, {.disableSignal = true});
|
||||
|
||||
EXPECT_CALL(
|
||||
network,
|
||||
ClientPublish(_, std::string_view{"foo"}, std::string_view{"double"},
|
||||
wpi::util::json::object(), IsDefaultPubSubOptions()));
|
||||
auto pub = storage.Publish(fooTopic, NT_DOUBLE, "double", {}, {});
|
||||
|
||||
auto val = Value::MakeDouble(1.0, 50);
|
||||
EXPECT_CALL(network, ClientSetValue(Handle{pub}.GetIndex(), val));
|
||||
storage.SetEntryValue(pub, val);
|
||||
|
||||
bool timedOut = false;
|
||||
EXPECT_FALSE(wpi::util::WaitForObject(sub, 0, &timedOut));
|
||||
EXPECT_TRUE(timedOut);
|
||||
}
|
||||
|
||||
class LocalStorageDuplicatesTest : public LocalStorageTest {
|
||||
public:
|
||||
void SetupPubSub(bool keepPub, bool keepSub);
|
||||
|
||||
@@ -28,6 +28,10 @@ bool PubSubOptionsMatcher::MatchAndExplain(
|
||||
*listener << "keepDuplicates mismatch ";
|
||||
match = false;
|
||||
}
|
||||
if (val.disableSignal != good.disableSignal) {
|
||||
*listener << "disableSignal mismatch ";
|
||||
match = false;
|
||||
}
|
||||
return match;
|
||||
}
|
||||
|
||||
|
||||
@@ -102,7 +102,8 @@ void PrintTo(const PubSubOptionsImpl& options, std::ostream* os) {
|
||||
*os << "PubSubOptions{periodicMs=" << options.periodicMs
|
||||
<< ", pollStorage=" << options.pollStorage
|
||||
<< ", sendAll=" << options.sendAll
|
||||
<< ", keepDuplicates=" << options.keepDuplicates << '}';
|
||||
<< ", keepDuplicates=" << options.keepDuplicates
|
||||
<< ", disableSignal=" << options.disableSignal << '}';
|
||||
}
|
||||
|
||||
} // namespace wpi::nt
|
||||
|
||||
@@ -190,7 +190,8 @@ RobotBase::RobotBase() {
|
||||
|
||||
auto inst = wpi::nt::NetworkTableInstance::GetDefault();
|
||||
// subscribe to "" to force persistent values to propagate to local
|
||||
wpi::nt::SubscribeMultiple(inst.GetHandle(), {{std::string_view{}}});
|
||||
wpi::nt::SubscribeMultiple(inst.GetHandle(), {{std::string_view{}}},
|
||||
{.disableSignal = true});
|
||||
if constexpr (!IsSimulation()) {
|
||||
inst.StartServer("/home/systemcore/networktables.json", "", "robot");
|
||||
} else {
|
||||
|
||||
@@ -166,7 +166,9 @@ class RobotStarter:
|
||||
inst = ntcore.NetworkTableInstance.getDefault()
|
||||
|
||||
# subscribe to "" to force persistent values to progagate to local
|
||||
msub = ntcore.MultiSubscriber(inst, [""])
|
||||
msub = ntcore.MultiSubscriber(
|
||||
inst, [""], ntcore.PubSubOptions(disableSignal=True)
|
||||
)
|
||||
|
||||
if not isSimulation:
|
||||
inst.startServer("/home/systemcore/networktables.json", "", "robot")
|
||||
|
||||
@@ -16,6 +16,7 @@ import org.wpilib.math.util.MathSharedStore;
|
||||
import org.wpilib.networktables.MultiSubscriber;
|
||||
import org.wpilib.networktables.NetworkTableEvent;
|
||||
import org.wpilib.networktables.NetworkTableInstance;
|
||||
import org.wpilib.networktables.PubSubOption;
|
||||
import org.wpilib.system.RuntimeType;
|
||||
import org.wpilib.system.Timer;
|
||||
import org.wpilib.system.WPILibVersion;
|
||||
@@ -103,7 +104,7 @@ public abstract class RobotBase implements AutoCloseable {
|
||||
setupCameraServerShared();
|
||||
setupMathShared();
|
||||
// subscribe to "" to force persistent values to propagate to local
|
||||
m_suball = new MultiSubscriber(inst, new String[] {""});
|
||||
m_suball = new MultiSubscriber(inst, new String[] {""}, PubSubOption.DISABLE_SIGNAL);
|
||||
if (!isSimulation()) {
|
||||
inst.startServer("/home/systemcore/networktables.json", "", "robot");
|
||||
} else {
|
||||
|
||||
Reference in New Issue
Block a user