[gdal-dev] GDAL Progressive Data Support RFC Discussion

[Frank Warmerdam]

Tamas Szekeres wrote:
> Frank,
> 
> The described approach with regards to the API seems reasonable for me, 
> I recall I took part in the conversation that time when the initial 
> version have been worked out, and I remember of most aspect.
> 
> Just by having a quick review of the code I have the following 
> questions/concerns:
> 
> 
> 1. The implementation of the PERSISTENT flag is handled by using a 
> static curl handle shared by multiple threads without having 
> synchronization in the cpl code. I'm not sure whether it's completely 
> guarded by the driver level mutex (pGlobalMutex), however it would be 
> better to do the sync in-place where the shared variable is declared. 
> This would require for example to place http_persistent_handle either at 
> the driver, or do the sync in the CPL code.
 >
> 2. Not sure whether it's safe to use the same curl handle by multiple 
> threads (either with syncing or without).

Tamas,

I agree that the current use of a persistent connection in the
CPL fetch code is unsafe.  I will look into correcting it today.
Also, this aspect is really not core to the API - just an artifact
needed for the JPIPKAK implementation so I have removed mention of
it from the RFC.

> 3. Could you explain the reason of using 2 worker threads (with 
> different priorities) at the driver? I'm not sure about the use case 
> where these 2 threads are required to have.

I'll have to let Norman or someone else at ITT respond to this
as I'm not clear on the use.  However, I will note that this is
an artifact of the JPIPKAK driver and not implicit in the
asynchronous IO API nor do I forsee it being a common pattern in
GDAL.

> 4. It would be reasonable to use (probably by extenting the CPL code) 
> sychronized waiting instead of busy-waiting 
> <http://en.wikipedia.org/wiki/Busy_waiting> implemented in the diver 
> code. This cause for example the implementation of 
> JPIPKAKAsyncRasterIO::Stop() getting odd, and error prone. For instance 
> it may result in a wait forever condition if the thread would terminate 
> without setting the exit flag correctly. By using a synchronized wait 
> approach instead (ie an event handle) the wait function would terminate 
> with an error if the owner thread have been terminated in the meantime.

I'm not entirely clear on what you mean about the current implementation
being error prone.  Perhaps you could provide some additional detail?

When you talk about a synchronized wait approach, I can imagine two
approach.

1) The application calls the POSIX select() function or a system
specific equivelent thing which blocks until there is data waiting on
one or more of the file/socket objects passed in.

2) A callback mechanism where a provided function is called when data
is ready - possible in a distinct thread, or as an interruption (ala
unix signals).

I am not fond of either approach as a universal mechanism because it
is often impractical to reengineer sublibraries to use them.  So for
instance, a driver like the OPeNDAP driver might use a separate library
to manage the connection to a remote server but it does not necessarly
provide any mechanism to implement either of the above methods.

So I am inclined to take a cheap polling appoach.  If an application
has several asynchronous activities on the go - perhaps GUI events,
socket connections and file io, it is easier to let it poll each
and then sleep/wait when there is nothing to do.

The defined RFC 24 API allows the application to do the waiting by
passing a timeout of zero into the GetNextUpdatedRegion() method.

I would add that the implementation of the timeout does *not*
do busy waiting.  Instead it uses CPLSleep() to release control
to the operating system for other processes between polls.

> 5. The diver doesn't seem to do any specific if CPLAcquireMutex fails or 
> returns with timeout (by not getting the ownership of the mutex in effect)

Good point.  This may be a flaw in the current driver which I can
try to rectify.  I will note that the existing JPIPKAK driver is
intended primarily to be an illustraction of the API in use rather
than a perfect implementation.  It was contributed by Norman and
adapted by me.

> By all means I have no problem with the API (which appears to be the 
> primary subject of this RFC)

Excellent!

Best regards,
-- 
---------------------------------------+--------------------------------------
I set the clouds in motion - turn up   | Frank Warmerdam, warmerdam at pobox.com
light and sound - activate the windows | http://pobox.com/~warmerdam
and watch the world go round - Rush    | Geospatial Programmer for Rent