Skip to content

Commit

Permalink
Change pointers to ordinary members in the FrameBuffer class
Browse files Browse the repository at this point in the history
The lifetime of m_buffer and m_intermediateBuffer aligns perfectly with
the FrameBuffer object. Thus, these lists should be ordinary members of
the FrameBuffer class - there is no need to store them as dynamically
alocated objects.
  • Loading branch information
dkozaratmobica committed Jul 26, 2017
1 parent 17d46f8 commit d428d9d
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 27 deletions.
46 changes: 21 additions & 25 deletions framebuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,13 @@
*
*/


#include <cstring>
#include "framebuffer.h"
#include "logging.h"

using namespace cannelloni;

FrameBuffer::FrameBuffer(size_t size, size_t max) :
m_buffer(new std::list<canfd_frame*>),
m_intermediateBuffer(new std::list<canfd_frame*>),
m_bufferSize(0),
m_intermediateBufferSize(0),
m_maxAllocCount(max),
Expand All @@ -38,8 +36,6 @@ FrameBuffer::FrameBuffer(size_t size, size_t max) :
FrameBuffer::~FrameBuffer() {
/* delete all frames */
clearPool();
delete m_buffer;
delete m_intermediateBuffer;
}

canfd_frame* FrameBuffer::requestFrame(bool overwriteLast, bool debug) {
Expand Down Expand Up @@ -97,7 +93,7 @@ void FrameBuffer::insertFramePool(canfd_frame *frame) {
void FrameBuffer::insertFrame(canfd_frame *frame) {
std::lock_guard<std::recursive_mutex> lock(m_bufferMutex);

m_buffer->push_back(frame);
m_buffer.push_back(frame);
m_bufferSize += CANNELLONI_FRAME_BASE_SIZE + canfd_len(frame);

/* We need one more byte for CAN_FD Frames */
Expand All @@ -108,7 +104,7 @@ void FrameBuffer::insertFrame(canfd_frame *frame) {
void FrameBuffer::returnFrame(canfd_frame *frame) {
std::lock_guard<std::recursive_mutex> lock(m_bufferMutex);

m_buffer->push_front(frame);
m_buffer.push_front(frame);
m_bufferSize += CANNELLONI_FRAME_BASE_SIZE + canfd_len(frame);
/* We need one more byte for CAN_FD Frames */
if (frame->len & CANFD_FRAME)
Expand All @@ -117,12 +113,12 @@ void FrameBuffer::returnFrame(canfd_frame *frame) {

canfd_frame* FrameBuffer::requestBufferFront() {
std::lock_guard<std::recursive_mutex> lock(m_bufferMutex);
if (m_buffer->empty()) {
if (m_buffer.empty()) {
return NULL;
}
else {
canfd_frame *ret = m_buffer->front();
m_buffer->pop_front();
canfd_frame *ret = m_buffer.front();
m_buffer.pop_front();
m_bufferSize -= (CANNELLONI_FRAME_BASE_SIZE + canfd_len(ret));
/* We need one more byte for CAN_FD Frames */
if (ret->len & CANFD_FRAME)
Expand All @@ -133,12 +129,12 @@ canfd_frame* FrameBuffer::requestBufferFront() {

canfd_frame* FrameBuffer::requestBufferBack() {
std::lock_guard<std::recursive_mutex> lock(m_bufferMutex);
if (m_buffer->empty()) {
if (m_buffer.empty()) {
return NULL;
}
else {
canfd_frame *ret = m_buffer->back();
m_buffer->pop_back();
canfd_frame *ret = m_buffer.back();
m_buffer.pop_back();
m_bufferSize -= (CANNELLONI_FRAME_BASE_SIZE + canfd_len(ret));
/* We need one more byte for CAN_FD Frames */
if (ret->len & CANFD_FRAME)
Expand All @@ -155,21 +151,21 @@ void FrameBuffer::swapBuffers() {
std::lock(lock1, lock2);

std::swap(m_bufferSize, m_intermediateBufferSize);
std::swap(m_buffer, m_intermediateBuffer);
m_buffer.swap(m_intermediateBuffer);
}

void FrameBuffer::sortIntermediateBuffer() {
std::lock_guard<std::recursive_mutex> lock(m_intermediateBufferMutex);

m_intermediateBuffer->sort(canfd_frame_comp());
m_intermediateBuffer.sort(canfd_frame_comp());
}

void FrameBuffer::mergeIntermediateBuffer() {
std::unique_lock<std::recursive_mutex> lock1(m_poolMutex, std::defer_lock);
std::unique_lock<std::recursive_mutex> lock2(m_intermediateBufferMutex, std::defer_lock);
std::lock(lock1, lock2);

m_framePool.splice(m_framePool.end(), *m_intermediateBuffer);
m_framePool.splice(m_framePool.end(), m_intermediateBuffer);
m_intermediateBufferSize = 0;
}

Expand All @@ -180,17 +176,17 @@ void FrameBuffer::returnIntermediateBuffer(std::list<canfd_frame*>::iterator sta

/* Don't splice since we need to keep track of the size */
for (std::list<canfd_frame*>::iterator it = start;
it != m_intermediateBuffer->end();) {
it != m_intermediateBuffer.end();) {
canfd_frame *frame = *it;
it = m_intermediateBuffer->erase(it);
it = m_intermediateBuffer.erase(it);
returnFrame(frame);
}
}

std::list<canfd_frame*>* FrameBuffer::getIntermediateBuffer() {
/* We need to lock m_intermediateBuffer here */
m_intermediateBufferMutex.lock();
return m_intermediateBuffer;
return &m_intermediateBuffer;
}

void FrameBuffer::unlockIntermediateBuffer() {
Expand All @@ -199,9 +195,9 @@ void FrameBuffer::unlockIntermediateBuffer() {

void FrameBuffer::debug() {
linfo << "FramePool: " << m_framePool.size() << std::endl;
linfo << "Buffer: " << m_buffer->size() << " (elements) "
linfo << "Buffer: " << m_buffer.size() << " (elements) "
<< m_bufferSize << " (bytes)" << std::endl;
linfo << "intermediateBuffer: " << m_intermediateBuffer->size() << std::endl;
linfo << "intermediateBuffer: " << m_intermediateBuffer.size() << std::endl;
}

void FrameBuffer::reset() {
Expand All @@ -211,8 +207,8 @@ void FrameBuffer::reset() {
std::lock(lock1, lock2, lock3);

/* Splice everything back into the pool */
m_framePool.splice(m_framePool.end(), *m_intermediateBuffer);
m_framePool.splice(m_framePool.end(), *m_buffer);
m_framePool.splice(m_framePool.end(), m_intermediateBuffer);
m_framePool.splice(m_framePool.end(), m_buffer);

m_intermediateBufferSize = 0;
m_bufferSize = 0;
Expand All @@ -227,10 +223,10 @@ void FrameBuffer::clearPool() {
for (canfd_frame *f : m_framePool) {
delete f;
}
for (canfd_frame *f : *m_intermediateBuffer) {
for (canfd_frame *f : m_intermediateBuffer) {
delete f;
}
for (canfd_frame *f : *m_buffer) {
for (canfd_frame *f : m_buffer) {
delete f;
}
m_framePool.clear();
Expand Down
4 changes: 2 additions & 2 deletions framebuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ class FrameBuffer {

private:
std::list<canfd_frame*> m_framePool;
std::list<canfd_frame*> *m_buffer;
std::list<canfd_frame*> *m_intermediateBuffer;
std::list<canfd_frame*> m_buffer;
std::list<canfd_frame*> m_intermediateBuffer;

uint64_t m_totalAllocCount;
/* When filling/swapping the buffers we currently need a mutex */
Expand Down

0 comments on commit d428d9d

Please sign in to comment.