From ea7e7a3637891f6d49e67f93ce0184371fa2ed50 Mon Sep 17 00:00:00 2001 From: Alejandro Homs Puron Date: Fri, 2 Feb 2024 21:36:03 +0100 Subject: [PATCH 1/2] Transform Buffer into a derivable class; remove (destroy) callback --- core/include/processlib/Data.h | 37 +++++++++++----------------------- 1 file changed, 12 insertions(+), 25 deletions(-) diff --git a/core/include/processlib/Data.h b/core/include/processlib/Data.h index 5064f21..00c37f0 100644 --- a/core/include/processlib/Data.h +++ b/core/include/processlib/Data.h @@ -46,25 +46,22 @@ struct DLL_EXPORT Buffer { - class DLL_EXPORT Callback - { - public: - virtual ~Callback() {} - virtual void destroy(void *dataPt) = 0; - }; - enum Ownership {MAPPED,SHARED}; - ~Buffer() + virtual ~Buffer() { - if(callback) - callback->destroy(data); + if(owner == SHARED && data) +#ifdef __unix + free(data); +#else + _aligned_free(data); +#endif pthread_mutex_destroy(&_lock); } - Buffer() : owner(SHARED),refcount(1),data(NULL),callback(NULL) + Buffer() : owner(SHARED),refcount(1),data(NULL) { pthread_mutex_init(&_lock,NULL); } - explicit Buffer(int aSize) :owner(SHARED),refcount(1),callback(NULL) + explicit Buffer(int aSize) :owner(SHARED),refcount(1) { #ifdef __unix if(posix_memalign(&data,16,aSize)) @@ -78,19 +75,10 @@ struct DLL_EXPORT Buffer void unref() { while(pthread_mutex_lock(&_lock)) ; - if(!(--refcount)) - { - if(owner == SHARED && data) -#ifdef __unix - free(data); -#else - _aligned_free(data); -#endif - pthread_mutex_unlock(&_lock); + bool destroy = (--refcount == 0); + pthread_mutex_unlock(&_lock); + if(destroy) delete this; - } - else - pthread_mutex_unlock(&_lock); } void ref() { @@ -102,7 +90,6 @@ struct DLL_EXPORT Buffer volatile int refcount; void *data; pthread_mutex_t _lock; - Callback* callback; }; struct DLL_EXPORT Data -- GitLab From 1e232e1e5ccd4aaa74f217c1a57ee40d7105255c Mon Sep 17 00:00:00 2001 From: Alejandro Homs Puron Date: Wed, 13 Mar 2024 11:20:51 +0100 Subject: [PATCH 2/2] Remove Buffer ownership, implement BufferBase, Buffer and MappedBuffer specific classes --- core/include/processlib/Data.h | 100 ++++++++++++++++++--------- tasks/include/processlib/Bpm.h | 10 +-- tasks/src/BackgroundSubstraction.cpp | 2 +- tasks/src/Bpm.cpp | 26 +++---- tasks/src/FlatfieldCorrection.cpp | 2 +- tasks/src/Mask.cpp | 3 +- 6 files changed, 90 insertions(+), 53 deletions(-) diff --git a/core/include/processlib/Data.h b/core/include/processlib/Data.h index 00c37f0..3c3dce9 100644 --- a/core/include/processlib/Data.h +++ b/core/include/processlib/Data.h @@ -38,40 +38,19 @@ #include #include #include +#include #include "processlib/Compatibility.h" #include "processlib/ProcessExceptions.h" #include "processlib/Container.h" #include "processlib/sideband/Data.h" -struct DLL_EXPORT Buffer +struct DLL_EXPORT BufferBase { - enum Ownership {MAPPED,SHARED}; - virtual ~Buffer() + virtual ~BufferBase() { - if(owner == SHARED && data) -#ifdef __unix - free(data); -#else - _aligned_free(data); -#endif pthread_mutex_destroy(&_lock); } - Buffer() : owner(SHARED),refcount(1),data(NULL) - { - pthread_mutex_init(&_lock,NULL); - } - explicit Buffer(int aSize) :owner(SHARED),refcount(1) - { -#ifdef __unix - if(posix_memalign(&data,16,aSize)) -#else /* window */ - data = _aligned_malloc(aSize,16); - if(!data) -#endif - std::cerr << "Can't allocate memory" << std::endl; - pthread_mutex_init(&_lock,NULL); - } void unref() { while(pthread_mutex_lock(&_lock)) ; @@ -86,10 +65,71 @@ struct DLL_EXPORT Buffer ++refcount; pthread_mutex_unlock(&_lock); } - Ownership owner; + virtual const char *type() const = 0; + volatile int refcount; void *data; pthread_mutex_t _lock; + + protected: + BufferBase(void *p = NULL) : refcount(1),data(p) + { + pthread_mutex_init(&_lock,NULL); + } +}; + +struct DLL_EXPORT Buffer : BufferBase +{ + virtual ~Buffer() + { +#ifdef __unix + free(data); +#else + _aligned_free(data); +#endif + } + + Buffer(int aSize) + { +#ifdef __unix + if(posix_memalign(&data,16,aSize)) +#else /* window */ + data = _aligned_malloc(aSize,16); + if(!data) +#endif + std::cerr << "Can't allocate memory" << std::endl; + } + + Buffer(void *p) : BufferBase(p) + { + } + + const char *type() const override + { + return "Standard"; + } +}; + +struct DLL_EXPORT MappedBuffer : BufferBase +{ + virtual ~MappedBuffer() + { + if (deleter) + deleter(data); + } + + template + MappedBuffer(void *p, D&& d) : BufferBase(p), deleter(std::forward(d)) + { + } + + const char *type() const override + { + return "Mapped"; + } + + protected: + std::function deleter; }; struct DLL_EXPORT Data @@ -115,7 +155,7 @@ struct DLL_EXPORT Data type = aData.type; dimensions = aData.dimensions; } - void setBuffer(Buffer *aBuffer) + void setBuffer(BufferBase *aBuffer) { if(aBuffer) aBuffer->ref(); if(buffer) buffer->unref(); @@ -267,7 +307,7 @@ struct DLL_EXPORT Data double timestamp; mutable HeaderContainer header; mutable SidebandContainer sideband; - mutable Buffer * buffer; + mutable BufferBase * buffer; private: template @@ -582,12 +622,10 @@ Data Data::cast(Data::TYPE aType) return aReturnData; } -inline std::ostream& operator<<(std::ostream &os,const Buffer &aBuffer) +inline std::ostream& operator<<(std::ostream &os,const BufferBase &aBuffer) { - const char *anOwnerShip = (aBuffer.owner == - Buffer::MAPPED) ? "Mapped" : "Shared"; os << "<" - << "owner=" << anOwnerShip << ", " + << "type=" << aBuffer.type() << ", " << "refcount=" << aBuffer.refcount << ", " << "data=" << aBuffer.data << ">"; diff --git a/tasks/include/processlib/Bpm.h b/tasks/include/processlib/Bpm.h index e7e8c24..2c98ef2 100644 --- a/tasks/include/processlib/Bpm.h +++ b/tasks/include/processlib/Bpm.h @@ -135,19 +135,19 @@ namespace Tasks template void _treat_image(const Data &aSrc, - Buffer &projection_x, - Buffer &projection_y, + BufferBase &projection_x, + BufferBase &projection_y, BpmResult &aResult); template void _tune_projection(const Data &aSrc, - Buffer&,Buffer&, + BufferBase&,BufferBase&, const BpmResult&); template - double _calculate_fwhm(const Buffer &projection,int size, + double _calculate_fwhm(const BufferBase &projection,int size, int max_index,double background_level, int &,int &); template - void _calculate_background(const Buffer &projection,double &background_level, + void _calculate_background(const BufferBase &projection,double &background_level, int min_index,int max_index); }; } diff --git a/tasks/src/BackgroundSubstraction.cpp b/tasks/src/BackgroundSubstraction.cpp index 8d12717..6e1188f 100644 --- a/tasks/src/BackgroundSubstraction.cpp +++ b/tasks/src/BackgroundSubstraction.cpp @@ -179,7 +179,7 @@ BackgroundSubstraction::BackgroundSubstraction(const BackgroundSubstraction &aBa void BackgroundSubstraction::setBackgroundImageData(Data &aData) { - if(aData.buffer && aData.buffer->owner == Buffer::MAPPED) + if(!aData.empty()) { Data copiedData = aData.copy(); _backgroundImageData.setData(copiedData); diff --git a/tasks/src/Bpm.cpp b/tasks/src/Bpm.cpp index 8f3b31c..fc164d1 100644 --- a/tasks/src/Bpm.cpp +++ b/tasks/src/Bpm.cpp @@ -57,12 +57,12 @@ template static inline INPUT min(INPUT a,INPUT b) */ template void BpmTask::_treat_image(const Data &aSrc, - Buffer &projection_x,Buffer &projection_y, + BufferBase &projection_x,BufferBase &projection_y, BpmResult &aResult) { SUMMTYPE *aProjectionX = (SUMMTYPE*)projection_x.data; SUMMTYPE *aProjectionY = (SUMMTYPE*)projection_y.data; - Buffer *aBufferPt = aSrc.buffer; + BufferBase *aBufferPt = aSrc.buffer; INPUT *aSrcPt = (INPUT*)aBufferPt->data; for(int y = 0;y < aSrc.dimensions[1];++y) for(int x = 0;x < aSrc.dimensions[0];++x) @@ -81,7 +81,7 @@ void BpmTask::_treat_image(const Data &aSrc, */ template void BpmTask::_tune_projection(const Data &aSrc, - Buffer &projection_x,Buffer &projection_y, + BufferBase &projection_x,BufferBase &projection_y, const BpmResult &aResult) { SUMMTYPE *aProjectionX = (SUMMTYPE*)projection_x.data; @@ -90,7 +90,7 @@ void BpmTask::_tune_projection(const Data &aSrc, SUMMTYPE *aProjectionY = (SUMMTYPE*)projection_y.data; memset(aProjectionY,0,aSrc.dimensions[1] * sizeof(SUMMTYPE)); - Buffer *aBufferPt = aSrc.buffer; + BufferBase *aBufferPt = aSrc.buffer; INPUT *aSrcPt = (INPUT*)aBufferPt->data; // X tuning profile @@ -122,7 +122,7 @@ void BpmTask::_tune_projection(const Data &aSrc, * average over three values and find the maximum in Y */ template -inline int _max_intensity_position(const Buffer &projection,int aSize) +inline int _max_intensity_position(const BufferBase &projection,int aSize) { int aMaxPos = -1; SUMMTYPE aMaxValue = 0; @@ -143,7 +143,7 @@ inline int _max_intensity_position(const Buffer &projection,int aSize) */ template static void _max_intensity(const Data &aSrc, - const Buffer &projection_x,const Buffer & projection_y, + const BufferBase &projection_x,const BufferBase & projection_y, BpmResult &aResult) { const int& width = aSrc.dimensions[0]; @@ -165,7 +165,7 @@ static void _max_intensity(const Data &aSrc, } if(xPositionFound && yPositionFound) { - Buffer *aBufferPt = aSrc.buffer; + BufferBase *aBufferPt = aSrc.buffer; INPUT *aSrcPt = (INPUT*)aBufferPt->data; // Center Pixel @@ -188,7 +188,7 @@ static void _max_intensity(const Data &aSrc, aResult.beam_intensity = -1.; } template -double BpmTask::_calculate_fwhm(const Buffer &projectionBuffer,int size, +double BpmTask::_calculate_fwhm(const BufferBase &projectionBuffer,int size, int peak_index,double background_level, int &min_index,int &max_index) { @@ -262,7 +262,7 @@ double BpmTask::_calculate_fwhm(const Buffer &projectionBuffer,int size, /** @brief Caluclate the background level around the peak */ template -void BpmTask::_calculate_background(const Buffer &projection,double &background_level, +void BpmTask::_calculate_background(const BufferBase &projection,double &background_level, int min_index,int max_index) { #if DEBUG @@ -394,9 +394,9 @@ static inline double _compute_center(double y[],int ly) } int error_flag = gsl_fft_complex_radix2_forward(data_n,1,n); - Buffer *dataN1 = new Buffer(2 * n1 * sizeof(double)); + BufferBase *dataN1 = new Buffer(2 * n1 * sizeof(double)); double *data_n1 = (double*)dataN1->data; - Buffer *yn1Re = new Buffer(n1 * sizeof(double)); + BufferBase *yn1Re = new Buffer(n1 * sizeof(double)); double *yn1_re = (double*)yn1Re->data; if (!error_flag) { @@ -558,7 +558,7 @@ BpmTask::BpmTask(const BpmTask &aTask) : min_index,max_index); \ /* calculate the beam center */ \ int size = max_index - min_index + 1; \ - Buffer *profile = new Buffer(size * sizeof(double)); \ + BufferBase *profile = new Buffer(size * sizeof(double)); \ double *aProfilePt = (double*)profile->data; \ SUMMTYPE *aSrcProfilePt = (SUMMTYPE*)projection_##XorY->data + min_index; \ for(int i = 0;i < size;++i) /* @todo optimized if needed */ \ @@ -649,7 +649,7 @@ void BpmTask::process(Data &aInputSrc) aResult.mBackgroundLevelTuney = aLastResult.mBackgroundLevelTuney; } aResult.frameNumber = aSrc.frameNumber; - Buffer *projection_x = NULL,*projection_y = NULL; + BufferBase *projection_x = NULL,*projection_y = NULL; switch(aSrc.type) { case Data::UINT8: diff --git a/tasks/src/FlatfieldCorrection.cpp b/tasks/src/FlatfieldCorrection.cpp index 5eda714..e2a14d2 100644 --- a/tasks/src/FlatfieldCorrection.cpp +++ b/tasks/src/FlatfieldCorrection.cpp @@ -113,7 +113,7 @@ void FlatfieldCorrection::setFlatFieldImageData(Data &aData,bool normalize) { if(normalize) FF = _normalize(aData); - else if(aData.buffer->owner == Buffer::MAPPED) + else if(!aData.empty()) FF = aData.copy(); else FF = aData; diff --git a/tasks/src/Mask.cpp b/tasks/src/Mask.cpp index 771784e..b15a483 100644 --- a/tasks/src/Mask.cpp +++ b/tasks/src/Mask.cpp @@ -35,8 +35,7 @@ Mask::Mask(const Mask &aTask) : void Mask::setMaskImageData(Data &aMaskImage) { - if(!aMaskImage.empty() && - aMaskImage.buffer->owner == Buffer::MAPPED) + if(!aMaskImage.empty()) { Data copiedData = aMaskImage.copy(); _MaskImage.setData(copiedData); -- GitLab