Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ticket #5272 - Avoid unnecesary memory allocations in OGR shapefile provider #26

Closed
wants to merge 8 commits into from
+245 −58
Closed

Ticket #5272 - Avoid unnecesary memory allocations in OGR shapefile provider #26

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
72a1ec3
#5272: Read object using optionally heap hooks
ahuarte47 Nov 14, 2013
ab40dea
#5272: Implements shared HeapHooks manager
ahuarte47 Nov 14, 2013
293c680
#5272: Use shared HeapHooks manager
ahuarte47 Nov 14, 2013
4457985
#5272: Fix bad size_t definition
ahuarte47 Nov 15, 2013
29b6d30
#5272: Refactoring code
ahuarte47 Nov 15, 2013
a2a111f
Merge remote-tracking branch 'upstream/1.10' into Ticket_5272-SHP
ahuarte47 Nov 15, 2013
7cdaf00
#5272: Use calloc instead malloc
ahuarte47 Nov 16, 2013
fd94956
#5272: fix doc comments
ahuarte47 Nov 16, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions gdal/ogr/ogrsf_frmts/shape/ogrshape.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@
/* ==================================================================== */
OGRFeature *SHPReadOGRFeature( SHPHandle hSHP, DBFHandle hDBF,
OGRFeatureDefn * poDefn, int iShape,
SHPObject *psShape, const char *pszSHPEncoding );
OGRGeometry *SHPReadOGRObject( SHPHandle hSHP, int iShape, SHPObject *psShape );
SHPObject *psShape, SAHeapObjectHooks *psHeapHooks, const char *pszSHPEncoding );
OGRGeometry *SHPReadOGRObject( SHPHandle hSHP, int iShape, SHPObject *psShape, SAHeapObjectHooks *psHeapHooks );
OGRFeatureDefn *SHPReadOGRFeatureDefn( const char * pszName,
SHPHandle hSHP, DBFHandle hDBF,
const char *pszSHPEncoding );
Expand All @@ -62,6 +62,7 @@ OGRErr SHPWriteOGRFeature( SHPHandle hSHP, DBFHandle hDBF,
/************************************************************************/

class OGRShapeDataSource;
class OGRShapeHeapObjectHooks;

class OGRShapeLayer : public OGRAbstractProxiedLayer
{
Expand Down Expand Up @@ -120,6 +121,8 @@ class OGRShapeLayer : public OGRAbstractProxiedLayer

void TruncateDBF();

OGRShapeHeapObjectHooks
*m_poHeapObjectHooks; /* Heap object manager for optimize memory allocations of pair 'SHPReadObjectH/SHPDestroyObjectH' */

protected:

Expand Down
124 changes: 107 additions & 17 deletions gdal/ogr/ogrsf_frmts/shape/ogrshapelayer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,89 @@

CPL_CVSID("$Id$");

/************************************************************************/
/* OGRShapeHeapObjectHooks() */
/* */
/* SAHeapObjectHooks provides a heap allocation mechanism for */
/* override the default malloc/free routines called in pair */
/* 'SHPReadObjectH/SHPDestroyObjectH' functions. */
/* */
/* We override this hook for share a memory buffer between the */
/* consecutive object readings and therefore optimize the memory */
/* management. */
/* The pair 'SHPReadObjectH/SHPDestroyObjectH' function must be */
/* called only once per object. */
/************************************************************************/

// Heap manager for optimize memory allocations in 'SHPReadObjectH/SHPDestroyObjectH'
class OGRShapeHeapObjectHooks : public SAHeapObjectHooks
{
public:
OGRShapeHeapObjectHooks() : mMemoryBuffer( NULL ), mMemorySize( 0 ), mLocked( false )
{
FCalloc = OGRShapeHeapObjectHooks::hook_calloc;
FFree = OGRShapeHeapObjectHooks::hook_free;
}
~OGRShapeHeapObjectHooks()
{
freeMem();
}
private:
void * mMemoryBuffer;
size_t mMemorySize;
bool mLocked;

// Free allocated memory
void freeMem()
{
if ( mMemoryBuffer )
{
OGRFree( mMemoryBuffer );
mMemoryBuffer = NULL;
mMemorySize = 0;
mLocked = false;
}
}

// Helper hook functions
static void *hook_calloc( void *thisHook, size_t num, size_t size )
{
return ((OGRShapeHeapObjectHooks*)thisHook)->sharedCalloc( num, size );
}
static void hook_free( void *thisHook, void *memblock )
{
return ((OGRShapeHeapObjectHooks*)thisHook)->sharedFree( memblock );
}

// Allocates shared memory blocks
void *sharedCalloc( size_t num, size_t size )
{
if ( mLocked )
throw "The pair 'SHPReadObjectH/SHPDestroyObjectH' must be called only once per object";

if ( mMemoryBuffer && mMemorySize < (num*size) )
freeMem();

if ( !mMemoryBuffer )
{
mMemoryBuffer = OGRCalloc( num, size );
mMemorySize = num * size;
}
else
{
memset( mMemoryBuffer, 0, num * size );
}
mLocked = true;

return mMemoryBuffer;
}
// Deallocates or frees shared memory blocks
void sharedFree( void *memblock )
{
mLocked = false;
}
};

/************************************************************************/
/* OGRShapeLayer() */
/************************************************************************/
Expand Down Expand Up @@ -132,6 +215,8 @@ OGRShapeLayer::OGRShapeLayer( OGRShapeDataSource* poDSIn,

poFeatureDefn = SHPReadOGRFeatureDefn( CPLGetBasename(pszFullName),
hSHP, hDBF, osEncoding );

m_poHeapObjectHooks = new OGRShapeHeapObjectHooks();
}

/************************************************************************/
Expand Down Expand Up @@ -175,6 +260,12 @@ OGRShapeLayer::~OGRShapeLayer()

if( hSBN != NULL )
SBNCloseDiskTree( hSBN );

if( m_poHeapObjectHooks )
{
delete m_poHeapObjectHooks;
m_poHeapObjectHooks = NULL;
}
}

/************************************************************************/
Expand Down Expand Up @@ -619,7 +710,7 @@ OGRFeature *OGRShapeLayer::FetchShape(int iShapeId /*, OGREnvelope* psShapeExten
{
SHPObject *psShape;

psShape = SHPReadObject( hSHP, iShapeId );
psShape = SHPReadObjectH( hSHP, iShapeId, m_poHeapObjectHooks );

// do not trust degenerate bounds on non-point geometries
// or bounds on null shapes.
Expand All @@ -632,14 +723,14 @@ OGRFeature *OGRShapeLayer::FetchShape(int iShapeId /*, OGREnvelope* psShapeExten
|| psShape->nSHPType == SHPT_NULL )
{
poFeature = SHPReadOGRFeature( hSHP, hDBF, poFeatureDefn,
iShapeId, psShape, osEncoding );
iShapeId, psShape, m_poHeapObjectHooks, osEncoding );
}
else if( m_sFilterEnvelope.MaxX < psShape->dfXMin
|| m_sFilterEnvelope.MaxY < psShape->dfYMin
|| psShape->dfXMax < m_sFilterEnvelope.MinX
|| psShape->dfYMax < m_sFilterEnvelope.MinY )
{
SHPDestroyObject(psShape);
SHPDestroyObjectH( psShape, m_poHeapObjectHooks );
poFeature = NULL;
}
else
Expand All @@ -649,13 +740,13 @@ OGRFeature *OGRShapeLayer::FetchShape(int iShapeId /*, OGREnvelope* psShapeExten
psShapeExtent->MaxX = psShape->dfXMax;
psShapeExtent->MaxY = psShape->dfYMax;*/
poFeature = SHPReadOGRFeature( hSHP, hDBF, poFeatureDefn,
iShapeId, psShape, osEncoding );
iShapeId, psShape, m_poHeapObjectHooks, osEncoding );
}
}
else
{
poFeature = SHPReadOGRFeature( hSHP, hDBF, poFeatureDefn,
iShapeId, NULL, osEncoding );
iShapeId, NULL, m_poHeapObjectHooks, osEncoding );
}

return poFeature;
Expand Down Expand Up @@ -759,7 +850,7 @@ OGRFeature *OGRShapeLayer::GetFeature( long nFeatureId )
return NULL;

OGRFeature *poFeature = NULL;
poFeature = SHPReadOGRFeature( hSHP, hDBF, poFeatureDefn, nFeatureId, NULL,
poFeature = SHPReadOGRFeature( hSHP, hDBF, poFeatureDefn, nFeatureId, NULL, m_poHeapObjectHooks,
osEncoding );

if( poFeature != NULL )
Expand Down Expand Up @@ -1040,7 +1131,7 @@ int OGRShapeLayer::GetFeatureCountWithSpatialFilterOnly()

/* Read full shape for point layers */
if (bExpectPoints)
psShape = SHPReadObject( hSHP, iShape);
psShape = SHPReadObjectH( hSHP, iShape, m_poHeapObjectHooks );

/* -------------------------------------------------------------------- */
/* Only read feature type and bounding box for now. In case of */
Expand Down Expand Up @@ -1092,10 +1183,10 @@ int OGRShapeLayer::GetFeatureCountWithSpatialFilterOnly()
/* We need to read the full geometry */
/* to compute the envelope */
if (psShape == &sShape)
psShape = SHPReadObject( hSHP, iShape);
psShape = SHPReadObjectH( hSHP, iShape, m_poHeapObjectHooks );
if (psShape)
{
poGeometry = SHPReadOGRObject( hSHP, iShape, psShape );
poGeometry = SHPReadOGRObject( hSHP, iShape, psShape, m_poHeapObjectHooks );
poGeometry->getEnvelope( &sGeomEnv );
psShape = NULL;
}
Expand Down Expand Up @@ -1145,11 +1236,10 @@ int OGRShapeLayer::GetFeatureCountWithSpatialFilterOnly()
if (poGeometry == NULL)
{
if (psShape == &sShape)
psShape = SHPReadObject( hSHP, iShape);
psShape = SHPReadObjectH( hSHP, iShape, m_poHeapObjectHooks );
if (psShape)
{
poGeometry =
SHPReadOGRObject( hSHP, iShape, psShape );
poGeometry = SHPReadOGRObject( hSHP, iShape, psShape, m_poHeapObjectHooks );
psShape = NULL;
}
}
Expand All @@ -1176,7 +1266,7 @@ int OGRShapeLayer::GetFeatureCountWithSpatialFilterOnly()
nFeatureCount ++;

if (psShape && psShape != &sShape)
SHPDestroyObject( psShape );
SHPDestroyObjectH( psShape, m_poHeapObjectHooks );
}

return nFeatureCount;
Expand Down Expand Up @@ -2313,14 +2403,14 @@ OGRErr OGRShapeLayer::Repack()
{
SHPObject *hObject;

hObject = SHPReadObject( hSHP, iShape );
hObject = SHPReadObjectH( hSHP, iShape, m_poHeapObjectHooks );
if( hObject == NULL )
eErr = OGRERR_FAILURE;
else if( SHPWriteObject( hNewSHP, -1, hObject ) == -1 )
eErr = OGRERR_FAILURE;

if( hObject )
SHPDestroyObject( hObject );
SHPDestroyObjectH( hObject, m_poHeapObjectHooks );
}
}

Expand Down Expand Up @@ -2580,7 +2670,7 @@ OGRErr OGRShapeLayer::RecomputeExtent()
{
if( hDBF == NULL || !DBFIsRecordDeleted( hDBF, iShape ) )
{
SHPObject *psObject = SHPReadObject( hSHP, iShape );
SHPObject *psObject = SHPReadObjectH( hSHP, iShape, m_poHeapObjectHooks );
if ( psObject != NULL &&
psObject->nSHPType != SHPT_NULL &&
psObject->nVertices != 0 )
Expand All @@ -2606,7 +2696,7 @@ OGRErr OGRShapeLayer::RecomputeExtent()
adBoundsMax[3] = MAX(adBoundsMax[3],psObject->padfM[i]);
}
}
SHPDestroyObject(psObject);
SHPDestroyObjectH( psObject, m_poHeapObjectHooks );
}
}

Expand Down
12 changes: 6 additions & 6 deletions gdal/ogr/ogrsf_frmts/shape/shape2ogr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,14 @@ static OGRLinearRing * CreateLinearRing ( SHPObject *psShape, int ring, int bHas
/* representation. */
/************************************************************************/

OGRGeometry *SHPReadOGRObject( SHPHandle hSHP, int iShape, SHPObject *psShape )
OGRGeometry *SHPReadOGRObject( SHPHandle hSHP, int iShape, SHPObject *psShape, SAHeapObjectHooks *psHeapHooks )
{
// CPLDebug( "Shape", "SHPReadOGRObject( iShape=%d )\n", iShape );

OGRGeometry *poOGR = NULL;

if( psShape == NULL )
psShape = SHPReadObject( hSHP, iShape );
psShape = SHPReadObjectH( hSHP, iShape, psHeapHooks );

if( psShape == NULL )
{
Expand Down Expand Up @@ -450,7 +450,7 @@ OGRGeometry *SHPReadOGRObject( SHPHandle hSHP, int iShape, SHPObject *psShape )
/* -------------------------------------------------------------------- */
/* Cleanup shape, and set feature id. */
/* -------------------------------------------------------------------- */
SHPDestroyObject( psShape );
SHPDestroyObjectH( psShape, psHeapHooks );

return poOGR;
}
Expand Down Expand Up @@ -983,7 +983,7 @@ OGRFeatureDefn *SHPReadOGRFeatureDefn( const char * pszName,

OGRFeature *SHPReadOGRFeature( SHPHandle hSHP, DBFHandle hDBF,
OGRFeatureDefn * poDefn, int iShape,
SHPObject *psShape, const char *pszSHPEncoding )
SHPObject *psShape, SAHeapObjectHooks *psHeapHooks, const char *pszSHPEncoding )

{
if( iShape < 0
Expand Down Expand Up @@ -1014,7 +1014,7 @@ OGRFeature *SHPReadOGRFeature( SHPHandle hSHP, DBFHandle hDBF,
if( !poDefn->IsGeometryIgnored() )
{
OGRGeometry* poGeometry = NULL;
poGeometry = SHPReadOGRObject( hSHP, iShape, psShape );
poGeometry = SHPReadOGRObject( hSHP, iShape, psShape, psHeapHooks );

/*
* NOTE - mloskot:
Expand All @@ -1029,7 +1029,7 @@ OGRFeature *SHPReadOGRFeature( SHPHandle hSHP, DBFHandle hDBF,
}
else if( psShape != NULL )
{
SHPDestroyObject( psShape );
SHPDestroyObjectH( psShape, psHeapHooks );
}
}

Expand Down
12 changes: 11 additions & 1 deletion gdal/ogr/ogrsf_frmts/shape/shapefil.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ static char *cvsid_aw() { return( cvsid_aw() ? ((char *) NULL) : cpl_cvsid ); }
#endif

/* -------------------------------------------------------------------- */
/* IO/Error hook functions. */
/* IO/Error/Heap hook functions. */
/* -------------------------------------------------------------------- */
typedef int *SAFile;

Expand All @@ -270,6 +270,12 @@ void SHPAPI_CALL SASetupDefaultHooks( SAHooks *psHooks );
void SHPAPI_CALL SASetupUtf8Hooks( SAHooks *psHooks );
#endif

// Support for override default routines to allocate and free memory in pair 'SHPReadObjectH/SHPDestroyObjectH'.
typedef struct {
void* (*FCalloc) ( void *thisHook, size_t num, size_t size );
void (*FFree) ( void *thisHook, void *memblock );
} SAHeapObjectHooks;

/************************************************************************/
/* SHP Support. */
/************************************************************************/
Expand Down Expand Up @@ -386,11 +392,15 @@ void SHPAPI_CALL

SHPObject SHPAPI_CALL1(*)
SHPReadObject( SHPHandle hSHP, int iShape );
SHPObject SHPAPI_CALL1(*)
SHPReadObjectH( SHPHandle hSHP, int iShape, SAHeapObjectHooks * psHeapHooks );
int SHPAPI_CALL
SHPWriteObject( SHPHandle hSHP, int iShape, SHPObject * psObject );

void SHPAPI_CALL
SHPDestroyObject( SHPObject * psObject );
void SHPAPI_CALL
SHPDestroyObjectH( SHPObject * psObject, SAHeapObjectHooks * psHeapHooks );
void SHPAPI_CALL
SHPComputeExtents( SHPObject * psObject );
SHPObject SHPAPI_CALL1(*)
Expand Down
Loading