Skip to content

Commit

Permalink
Merge pull request #8969 from rouault/fix_8967
Browse files Browse the repository at this point in the history
VRTComplexSource: fix excessive RAM usage with many sources (fixes #8967, 3.8.0 regression)
  • Loading branch information
rouault authored Dec 16, 2023
2 parents e324eb3 + 94228b6 commit b5d004f
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 70 deletions.
4 changes: 3 additions & 1 deletion frmts/postgisraster/postgisrasterrasterband.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,7 @@ CPLErr PostGISRasterRasterBand::IRasterIO(
sizeof(PostGISRasterTileDataset *), SortTilesByPKID);
}

VRTSource::WorkingState oWorkingState;
for (i = 0; i < nFeatureCount && eErr == CE_None; i++)
{
PostGISRasterTileDataset *poTile = papsMatchingTiles[i];
Expand All @@ -617,7 +618,8 @@ CPLErr PostGISRasterRasterBand::IRasterIO(
poTile->GetRasterBand(nBand));
eErr = poTileBand->poSource->RasterIO(
eDataType, nXOff, nYOff, nXSize, nYSize, pData, nBufXSize,
nBufYSize, eBufType, nPixelSpace, nLineSpace, nullptr);
nBufYSize, eBufType, nPixelSpace, nLineSpace, nullptr,
oWorkingState);
}

// Free the object that holds pointers to matching tiles
Expand Down
72 changes: 42 additions & 30 deletions frmts/vrt/vrtdataset.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,39 @@ class VRTOverviewInfo
class CPL_DLL VRTSource
{
public:
struct CPL_DLL WorkingState
{
// GByte whose initialization constructor does nothing
#ifdef __GNUC__
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Weffc++"
#endif
struct NoInitByte
{
GByte value;
// cppcheck-suppress uninitMemberVar
NoInitByte()
{
// do nothing
/* coverity[uninit_member] */
}
};
#ifdef __GNUC__
#pragma GCC diagnostic pop
#endif

std::vector<NoInitByte> m_abyWrkBuffer{};
std::vector<NoInitByte> m_abyWrkBufferMask{};
};

virtual ~VRTSource();

virtual CPLErr RasterIO(GDALDataType eVRTBandDataType, int nXOff, int nYOff,
int nXSize, int nYSize, void *pData, int nBufXSize,
int nBufYSize, GDALDataType eBufType,
GSpacing nPixelSpace, GSpacing nLineSpace,
GDALRasterIOExtraArg *psExtraArg) = 0;
GDALRasterIOExtraArg *psExtraArg,
WorkingState &oWorkingState) = 0;

virtual double GetMinimum(int nXSize, int nYSize, int *pbSuccess) = 0;
virtual double GetMaximum(int nXSize, int nYSize, int *pbSuccess) = 0;
Expand Down Expand Up @@ -213,6 +239,8 @@ class CPL_DLL VRTDataset CPL_NON_FINAL : public GDALDataset
std::map<CPLString, GDALDataset *> m_oMapSharedSources{};
std::shared_ptr<VRTGroup> m_poRootGroup{};

VRTSource::WorkingState m_oWorkingState{};

VRTRasterBand *InitBand(const char *pszSubclass, int nBand,
bool bAllowPansharpened);
static GDALDataset *OpenVRTProtocol(const char *pszSpec);
Expand Down Expand Up @@ -1072,7 +1100,8 @@ class CPL_DLL VRTSimpleSource CPL_NON_FINAL : public VRTSource
int nXSize, int nYSize, void *pData, int nBufXSize,
int nBufYSize, GDALDataType eBufType,
GSpacing nPixelSpace, GSpacing nLineSpace,
GDALRasterIOExtraArg *psExtraArgIn) override;
GDALRasterIOExtraArg *psExtraArgIn,
WorkingState &oWorkingState) override;

virtual double GetMinimum(int nXSize, int nYSize, int *pbSuccess) override;
virtual double GetMaximum(int nXSize, int nYSize, int *pbSuccess) override;
Expand Down Expand Up @@ -1133,7 +1162,8 @@ class VRTAveragedSource final : public VRTSimpleSource
int nXSize, int nYSize, void *pData, int nBufXSize,
int nBufYSize, GDALDataType eBufType,
GSpacing nPixelSpace, GSpacing nLineSpace,
GDALRasterIOExtraArg *psExtraArgIn) override;
GDALRasterIOExtraArg *psExtraArgIn,
WorkingState &oWorkingState) override;

virtual double GetMinimum(int nXSize, int nYSize, int *pbSuccess) override;
virtual double GetMaximum(int nXSize, int nYSize, int *pbSuccess) override;
Expand Down Expand Up @@ -1172,28 +1202,6 @@ class CPL_DLL VRTComplexSource CPL_NON_FINAL : public VRTSimpleSource

int m_nProcessingFlags = 0;

// GByte whose initialization constructor does nothing
#ifdef __GNUC__
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Weffc++"
#endif
struct NoInitByte
{
GByte value;
// cppcheck-suppress uninitMemberVar
NoInitByte()
{
// do nothing
/* coverity[uninit_member] */
}
};
#ifdef __GNUC__
#pragma GCC diagnostic pop
#endif

std::vector<NoInitByte> m_abyWrkBuffer{};
std::vector<NoInitByte> m_abyWrkBufferMask{};

// adjusted value should be read with GetAdjustedNoDataValue()
double m_dfNoDataValue = VRT_NODATA_UNSET;
std::string
Expand Down Expand Up @@ -1224,7 +1232,7 @@ class CPL_DLL VRTComplexSource CPL_NON_FINAL : public VRTSimpleSource
int nReqXSize, int nReqYSize, void *pData, int nOutXSize,
int nOutYSize, GDALDataType eBufType, GSpacing nPixelSpace,
GSpacing nLineSpace, GDALRasterIOExtraArg *psExtraArg,
GDALDataType eWrkDataType);
GDALDataType eWrkDataType, WorkingState &oWorkingState);

template <class SourceDT, GDALDataType eSourceType>
CPLErr RasterIOProcessNoData(GDALRasterBand *poSourceBand,
Expand All @@ -1233,7 +1241,8 @@ class CPL_DLL VRTComplexSource CPL_NON_FINAL : public VRTSimpleSource
void *pData, int nOutXSize, int nOutYSize,
GDALDataType eBufType, GSpacing nPixelSpace,
GSpacing nLineSpace,
GDALRasterIOExtraArg *psExtraArg);
GDALRasterIOExtraArg *psExtraArg,
WorkingState &oWorkingState);

public:
VRTComplexSource() = default;
Expand All @@ -1244,7 +1253,8 @@ class CPL_DLL VRTComplexSource CPL_NON_FINAL : public VRTSimpleSource
int nXSize, int nYSize, void *pData, int nBufXSize,
int nBufYSize, GDALDataType eBufType,
GSpacing nPixelSpace, GSpacing nLineSpace,
GDALRasterIOExtraArg *psExtraArgIn) override;
GDALRasterIOExtraArg *psExtraArgIn,
WorkingState &oWorkingState) override;

virtual double GetMinimum(int nXSize, int nYSize, int *pbSuccess) override;
virtual double GetMaximum(int nXSize, int nYSize, int *pbSuccess) override;
Expand Down Expand Up @@ -1313,7 +1323,8 @@ class VRTFilteredSource CPL_NON_FINAL : public VRTComplexSource
int nXSize, int nYSize, void *pData, int nBufXSize,
int nBufYSize, GDALDataType eBufType,
GSpacing nPixelSpace, GSpacing nLineSpace,
GDALRasterIOExtraArg *psExtraArg) override;
GDALRasterIOExtraArg *psExtraArg,
WorkingState &oWorkingState) override;
};

/************************************************************************/
Expand Down Expand Up @@ -1387,7 +1398,8 @@ class VRTFuncSource final : public VRTSource
int nXSize, int nYSize, void *pData, int nBufXSize,
int nBufYSize, GDALDataType eBufType,
GSpacing nPixelSpace, GSpacing nLineSpace,
GDALRasterIOExtraArg *psExtraArg) override;
GDALRasterIOExtraArg *psExtraArg,
WorkingState &oWorkingState) override;

virtual double GetMinimum(int nXSize, int nYSize, int *pbSuccess) override;
virtual double GetMaximum(int nXSize, int nYSize, int *pbSuccess) override;
Expand Down
3 changes: 2 additions & 1 deletion frmts/vrt/vrtderivedrasterband.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1125,6 +1125,7 @@ CPLErr VRTDerivedRasterBand::IRasterIO(

// Load values for sources into packed buffers.
CPLErr eErr = CE_None;
VRTSource::WorkingState oWorkingState;
for (int iBuffer = 0; iBuffer < nBufferCount && eErr == CE_None; iBuffer++)
{
const int iSource = anMapBufferIdxToSourceIdx[iBuffer];
Expand All @@ -1136,7 +1137,7 @@ CPLErr VRTDerivedRasterBand::IRasterIO(
(nYShiftInBuffer * nExtBufXSize + nXShiftInBuffer) *
nSrcTypeSize,
nExtBufXSizeReq, nExtBufYSizeReq, eSrcType, nSrcTypeSize,
nSrcTypeSize * nExtBufXSize, &sExtraArg);
nSrcTypeSize * nExtBufXSize, &sExtraArg, oWorkingState);

// Extend first lines
for (int iY = 0; iY < nYShiftInBuffer; iY++)
Expand Down
9 changes: 6 additions & 3 deletions frmts/vrt/vrtfilters.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ CPLErr VRTFilteredSource::RasterIO(GDALDataType eVRTBandDataType, int nXOff,
void *pData, int nBufXSize, int nBufYSize,
GDALDataType eBufType, GSpacing nPixelSpace,
GSpacing nLineSpace,
GDALRasterIOExtraArg *psExtraArg)
GDALRasterIOExtraArg *psExtraArg,
WorkingState &oWorkingState)

{
/* -------------------------------------------------------------------- */
Expand All @@ -140,7 +141,8 @@ CPLErr VRTFilteredSource::RasterIO(GDALDataType eVRTBandDataType, int nXOff,
{
return VRTComplexSource::RasterIO(
eVRTBandDataType, nXOff, nYOff, nXSize, nYSize, pData, nBufXSize,
nBufYSize, eBufType, nPixelSpace, nLineSpace, psExtraArg);
nBufYSize, eBufType, nPixelSpace, nLineSpace, psExtraArg,
oWorkingState);
}

double dfXOff = nXOff;
Expand Down Expand Up @@ -321,7 +323,8 @@ CPLErr VRTFilteredSource::RasterIO(GDALDataType eVRTBandDataType, int nXOff,
nFileYSize,
pabyWorkData + nLineOffset * nTopFill + nPixelOffset * nLeftFill,
nFileXSize, nFileYSize, eOperDataType, nPixelOffset, nLineOffset,
&sExtraArgs, bIsComplex ? GDT_CFloat32 : GDT_Float32);
&sExtraArgs, bIsComplex ? GDT_CFloat32 : GDT_Float32,
oWorkingState);

if (eErr != CE_None)
{
Expand Down
14 changes: 8 additions & 6 deletions frmts/vrt/vrtmultidim.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2602,8 +2602,8 @@ class VRTArraySource : public VRTSource
CPLErr RasterIO(GDALDataType eBandDataType, int nXOff, int nYOff,
int nXSize, int nYSize, void *pData, int nBufXSize,
int nBufYSize, GDALDataType eBufType, GSpacing nPixelSpace,
GSpacing nLineSpace,
GDALRasterIOExtraArg *psExtraArg) override;
GSpacing nLineSpace, GDALRasterIOExtraArg *psExtraArg,
WorkingState &oWorkingState) override;

double GetMinimum(int nXSize, int nYSize, int *pbSuccess) override
{
Expand Down Expand Up @@ -2641,11 +2641,13 @@ CPLErr VRTArraySource::RasterIO(GDALDataType eBandDataType, int nXOff,
int nBufXSize, int nBufYSize,
GDALDataType eBufType, GSpacing nPixelSpace,
GSpacing nLineSpace,
GDALRasterIOExtraArg *psExtraArg)
GDALRasterIOExtraArg *psExtraArg,
WorkingState &oWorkingState)
{
return m_poSimpleSource->RasterIO(
eBandDataType, nXOff, nYOff, nXSize, nYSize, pData, nBufXSize,
nBufYSize, eBufType, nPixelSpace, nLineSpace, psExtraArg);
return m_poSimpleSource->RasterIO(eBandDataType, nXOff, nYOff, nXSize,
nYSize, pData, nBufXSize, nBufYSize,
eBufType, nPixelSpace, nLineSpace,
psExtraArg, oWorkingState);
}

/************************************************************************/
Expand Down
4 changes: 3 additions & 1 deletion frmts/vrt/vrtsourcedrasterband.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,7 @@ CPLErr VRTSourcedRasterBand::IRasterIO(
/* Overlay each source in turn over top this. */
/* -------------------------------------------------------------------- */
CPLErr eErr = CE_None;
VRTSource::WorkingState oWorkingState;
for (int iSource = 0; eErr == CE_None && iSource < nSources; iSource++)
{
psExtraArg->pfnProgress = GDALScaledProgress;
Expand All @@ -369,7 +370,8 @@ CPLErr VRTSourcedRasterBand::IRasterIO(

eErr = papoSources[iSource]->RasterIO(
eDataType, nXOff, nYOff, nXSize, nYSize, pData, nBufXSize,
nBufYSize, eBufType, nPixelSpace, nLineSpace, psExtraArg);
nBufYSize, eBufType, nPixelSpace, nLineSpace, psExtraArg,
l_poDS ? l_poDS->m_oWorkingState : oWorkingState);

GDALDestroyScaledProgress(psExtraArg->pProgressData);
}
Expand Down
Loading

0 comments on commit b5d004f

Please sign in to comment.