Remove std::memcpy while processing data in compressor task #5302
Conversation
preghenella
force-pushed
the
rdev-tofcompressor-nomemcpy
branch
from
beac10d
to
516680c
Compare
|
ciao @noferini |
| auto headerIn = DataRefUtils::getHeader<o2::header::DataHeader*>(ref); | ||
| auto dataProcessingHeaderIn = DataRefUtils::getHeader<o2::framework::DataProcessingHeader*>(ref); | ||
| auto payloadIn = ref.payload; | ||
| auto payloadInSize = headerIn->payloadSize; | ||
|
|
||
| /** prepare **/ | ||
| auto bufferSize = mOutputBufferSize > 0 ? mOutputBufferSize : payloadInSize; |
There was a problem hiding this comment.
@preghenella in which situation you would provide explicitly the buffer size, rather then relying on automatic one?
Isn't this prone providing too small size?
There was a problem hiding this comment.
@shahor02
normal operation in principle better works with an explicit buffer size (via cmdline argument) in case the automatic size is found too small.
it can indeed come with risk of providing too small size, but we will check carefully (and ERRORS will be issued if too small)
There was a problem hiding this comment.
btw let me add that this PR is part of an incremental update.
eventually we want to try to remove creating these many messages (one for each input part) and possibly lower the number of shm allocations.
This will mean that the automatic buffer allocation will not be usable anymore (we do not know how large are the multi-part messages in beforehand) and we must rely on setting externally a buffer which is large enough to accommodate the output data.
There was a problem hiding this comment.
@preghenella thanks for explanations. Isn't output size guaranteed to be smaller than the input, in which case the input payload size would be a safe bet? As for merging multiple input parts into 1 output: since all inputs headers at available, one can estimate the output size upper limit in a same way as for single part.
There was a problem hiding this comment.
yes, the automatic buffer size is indeed a safe bet (currently it is the default setting in fact).
re. the multi-part, you are probably right and an automatic buffer can be done also in that case.
maybe in the future we can just remove the possibility to define the buffer size from command line.
but for the time being, given that we have no freedom during tests on the FLP to update code on a quick basis, we keep it such that we have an adaptable soluion
There was a problem hiding this comment.
Sure, wanted just to understand if it is really necessary.
This PR removes an unnecessary
std::memcpywhile running the TOF compressor device.The buffer for the message is created before hand and used size adjusted according to the output to be forwarded downstream.