PcloudStimProd: make sh_ptrs to Pcloud*StimBuff atomic<>

This change is a bit pedantic, but since these vars aren't accessed
in any hotpath, it's fine to be pedantic. We made these sh_ptrs
atomic so we can use acquire and release side effects when loading
and storing them. This doesn't eliminate the problem of seeing
inconsistent state across microcontrollers, but it helps with simple
accesses like these ones we already do.
This commit is contained in:
2025-11-26 00:11:40 -04:00
parent edab71b823
commit 5cce473e01
4 changed files with 46 additions and 31 deletions
@@ -650,9 +650,14 @@ bool OpenClCollatingAndMeshingEngine::setupCollateDgramsArgs(
// Set ambienceHighVal argument (arg 4) // Set ambienceHighVal argument (arg 4)
uint32_t ambienceHighVal = 0; uint32_t ambienceHighVal = 0;
if (ambienceStimFrame.has_value() && parent.ambienceStimulusBuffer) { std::shared_ptr<PcloudAmbienceStimulusBuffer> ambienceBuff = nullptr;
ambienceHighVal = parent.ambienceStimulusBuffer->ambienceHighVal; if (ambienceStimFrame.has_value()
&& (ambienceBuff = parent.ambienceStimulusBuffer.load(
std::memory_order_acquire)))
{
ambienceHighVal = ambienceBuff->ambienceHighVal;
} }
err = clSetKernelArg(collateKernel, 4, sizeof(uint32_t), &ambienceHighVal); err = clSetKernelArg(collateKernel, 4, sizeof(uint32_t), &ambienceHighVal);
if (err != CL_SUCCESS) if (err != CL_SUCCESS)
{ {
@@ -214,12 +214,24 @@ void PcloudStimulusProducer::destroyAttachedStimulusBuffer(
this->stop(); this->stop();
// Clear specialized buffer pointers if they match // Clear specialized buffer pointers if they match
if (meshStimulusBuffer == buffer) auto meshBuff = meshStimulusBuffer.load(std::memory_order_acquire);
{ meshStimulusBuffer.reset(); } if (meshBuff == buffer)
if (intensityStimulusBuffer == buffer) {
{ intensityStimulusBuffer.reset(); } meshBuff.reset();
if (ambienceStimulusBuffer == buffer) meshStimulusBuffer.store(nullptr, std::memory_order_release);
{ ambienceStimulusBuffer.reset(); } }
auto intensityBuff = intensityStimulusBuffer.load(std::memory_order_acquire);
if (intensityBuff == buffer)
{
intensityBuff.reset();
intensityStimulusBuffer.store(nullptr, std::memory_order_release);
}
auto ambienceBuff = ambienceStimulusBuffer.load(std::memory_order_acquire);
if (ambienceBuff == buffer)
{
ambienceBuff.reset();
ambienceStimulusBuffer.store(nullptr, std::memory_order_release);
}
// Call base class implementation to remove from attachedStimulusBuffers // Call base class implementation to remove from attachedStimulusBuffers
StimulusProducer::destroyAttachedStimulusBuffer(buffer); StimulusProducer::destroyAttachedStimulusBuffer(buffer);
@@ -266,7 +278,7 @@ std::cout << __func__ << ": $$$$$$$ Creating MeshStimulusBuffer" << std::endl;
std::cout << __func__ << ": $$$$$$$ Created MeshStimulusBuffer" << std::endl; std::cout << __func__ << ": $$$$$$$ Created MeshStimulusBuffer" << std::endl;
this->stop(); this->stop();
addAttachedStimulusBufferIfNotExists(meshBuffer); addAttachedStimulusBufferIfNotExists(meshBuffer);
meshStimulusBuffer = meshBuffer; meshStimulusBuffer.store(meshBuffer, std::memory_order_release);
this->start(); this->start();
return meshBuffer; return meshBuffer;
} }
@@ -289,7 +301,8 @@ std::cout << __func__ << ": $$$$$$$ Creating PcloudIntensityStimulusBuffer" << s
std::cout << __func__ << ": $$$$$$$ Created PcloudIntensityStimulusBuffer" << std::endl; std::cout << __func__ << ": $$$$$$$ Created PcloudIntensityStimulusBuffer" << std::endl;
this->stop(); this->stop();
addAttachedStimulusBufferIfNotExists(intensityBuffer); addAttachedStimulusBufferIfNotExists(intensityBuffer);
intensityStimulusBuffer = intensityBuffer; intensityStimulusBuffer.store(
intensityBuffer, std::memory_order_release);
this->start(); this->start();
return intensityBuffer; return intensityBuffer;
} }
@@ -322,7 +335,7 @@ std::cout << __func__ << ": $$$$$$$ Created PcloudIntensityStimulusBuffer" << st
std::cout << __func__ << ": $$$$$$$ Created PcloudAmbienceStimulusBuffer" << std::endl; std::cout << __func__ << ": $$$$$$$ Created PcloudAmbienceStimulusBuffer" << std::endl;
this->stop(); this->stop();
addAttachedStimulusBufferIfNotExists(ambienceBuffer); addAttachedStimulusBufferIfNotExists(ambienceBuffer);
ambienceStimulusBuffer = ambienceBuffer; ambienceStimulusBuffer.store(ambienceBuffer, std::memory_order_release);
this->start(); this->start();
return ambienceBuffer; return ambienceBuffer;
} }
@@ -407,13 +420,14 @@ public:
context->frameAssemblyResult = loop; context->frameAssemblyResult = loop;
// Check if intensity buffer is attached and acquire frame if so // Check if intensity buffer is attached and acquire frame if so
if (pcloudProducer.intensityStimulusBuffer) if (auto intensityBuff = pcloudProducer.intensityStimulusBuffer.load(
std::memory_order_acquire))
{ {
size_t intensityRingbuffIndex = pcloudProducer size_t intensityRingbuffIndex = intensityBuff
.intensityStimulusBuffer->ringBuffer.getIndexToProduceInto(); ->ringBuffer.getIndexToProduceInto();
StimulusFrame& intensityStimFrame = pcloudProducer StimulusFrame& intensityStimFrame = intensityBuff
.intensityStimulusBuffer->ringBuffer.getDataAtSlot( ->ringBuffer.getDataAtSlot(
intensityRingbuffIndex); intensityRingbuffIndex);
intensityStimFrame.lock.writeAcquire(); intensityStimFrame.lock.writeAcquire();
@@ -425,13 +439,14 @@ public:
} }
// Check if ambience buffer is attached and acquire frame if so // Check if ambience buffer is attached and acquire frame if so
if (pcloudProducer.ambienceStimulusBuffer) if (auto ambienceBuff = pcloudProducer.ambienceStimulusBuffer.load(
std::memory_order_acquire))
{ {
size_t ambienceRingbuffIndex = pcloudProducer size_t ambienceRingbuffIndex = ambienceBuff
.ambienceStimulusBuffer->ringBuffer.getIndexToProduceInto(); ->ringBuffer.getIndexToProduceInto();
StimulusFrame& ambienceStimFrame = pcloudProducer StimulusFrame& ambienceStimFrame = ambienceBuff
.ambienceStimulusBuffer->ringBuffer.getDataAtSlot( ->ringBuffer.getDataAtSlot(
ambienceRingbuffIndex); ambienceRingbuffIndex);
ambienceStimFrame.lock.writeAcquire(); ambienceStimFrame.lock.writeAcquire();
@@ -91,9 +91,11 @@ public:
StagingBuffer collationBuffer; StagingBuffer collationBuffer;
size_t tempStimulusFrameMem; size_t tempStimulusFrameMem;
StimulusFrame tempStimulusFrame; StimulusFrame tempStimulusFrame;
std::shared_ptr<MeshStimulusBuffer> meshStimulusBuffer; std::atomic<std::shared_ptr<MeshStimulusBuffer>> meshStimulusBuffer;
std::shared_ptr<PcloudIntensityStimulusBuffer> intensityStimulusBuffer; std::atomic<std::shared_ptr<PcloudIntensityStimulusBuffer>>
std::shared_ptr<PcloudAmbienceStimulusBuffer> ambienceStimulusBuffer; intensityStimulusBuffer;
std::atomic<std::shared_ptr<PcloudAmbienceStimulusBuffer>>
ambienceStimulusBuffer;
private: private:
class ProduceFrameReq; class ProduceFrameReq;
-7
View File
@@ -99,10 +99,3 @@ cancelation problem described above, concerning
StimulusBuffer::start/stop(), and ensuring that after StimulusBuffer::start/stop(), and ensuring that after
stop() has returned, we can be reasonably sure that all stop() has returned, we can be reasonably sure that all
in-flight ops have exited. in-flight ops have exited.
Making sh_ptr<StimulusBuffer> atomic for mem barriers:
We could also complete our implemetation's correctness by converting
the sh_ptrs to StimulusBuffer inside of the PCloudStimulusProducer
into std::atomic<std::shared_ptr<StimulusBuffer>>, and using
std::memory_order_release/memory_order_acquire when writing and
reading them respectively.