<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<br>
<blockquote type="cite"
cite="mid:CA+YzLBeoC4qx8dSxe8s_TV5if8+AJf_do3qs8Du_LLkRLVSLGg@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<div><br>
</div>
<div class="gmail_default"
style="font-family:arial,sans-serif">For read-only purposes
I would have thought we only had to briefly seize a mutex
when introducing new blocks into the cache, and I thought
there was already some sort of lock around that. <br>
</div>
</div>
</div>
</blockquote>
<p>When talking about the block cache, one needs to distinguish:</p>
<p>- the global least-recently-used (LRU) list of GDALRasterBlock*
pointers, used by all datasets, that has indeed a lock since it is
used already in a multi-threaded context</p>
<p>- GDALRasterBlock which have an atomic counter aimed at
coordinating this LRU list with the below mentionned per-band data
structures, to avoid in particular eviction from the LRU due to
the max size of the block cache being reached and dataset closing
to badly interfere<br>
</p>
<p>- per-band data structures ( either as an array of array, or a
hash-set (for very large datasets)) that own GDALRasterBlock*
instances, Those don't do any locking around access to those
structures. Well, actually, careful observers would object that
I'm lying here. There is a "secret" mutex per GDALDataset*
instance, used currently only for datasets opened in update mode
and taken in the non-virtual RasterIO() method, that addresses
complex cases where the LRU logic would try to evict from a thread
T2 a "dirty" block, causing IWriteBlock() to be called, while a
thread T1 would IReadBlock() on the same dataset. That mutex
actually leads to "interesting" issues with deadlocks with the
mutex of the global LRU, which lead to some subtelties in the
GetLockedBlockRef() implementation (cf lines 1960-1983 of
gdalrasterband.cpp) Which would lead to another interesting
discussion on the fact that for scenarios where multi-threaded
writing of *different dataset* the current architecture of a
global LRU is inefficient and would need to probably be replaced
by a per-dataset LRU.</p>
<p>I agree that the per-band data structures could be made
thread-safe with careful use of a mutex. One tricky point would
remain if 2 threads try to access at the same time a block that
hasn't been loaded yet in the cache (although an algorithm fully
optimized for threading should really try to avoid accessing the
same block from multiple threads)<br>
</p>
<p>2 possible strategies:</p>
<p>- each thread actually IReadBlock() (assuming that its
implementation is thread-safe) the block and at the time they call
AdoptBlock(), insertion in the data structure is locked, and only
the first "version" of GDALRasterBlock is kept, and the one that
arrives later is deleted. But one should make sure there's no risk
of use-after-free.</p>
<p>- a more complex mechanism is used so that it is "noted
somewhere" that block (xoff, yoff) is in loading and that the
thread that arrives second is paused upon completion of the
loading. That solution is more respectful of total CPU power
consumption, but maybe less efficient in term of throughput.</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:CA+YzLBeoC4qx8dSxe8s_TV5if8+AJf_do3qs8Du_LLkRLVSLGg@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<div class="gmail_default"
style="font-family:arial,sans-serif"><br>
</div>
<blockquote class="gmail_quote"
style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr">
<div style="font-family:arial,sans-serif"> - no
duplication of file handles per-thread</div>
</div>
</div>
</blockquote>
<p>I just don't see how to do that. Even a mutex around
file operations would not be sufficient since we'd want
both Seek()+Read() to be atomic per thread. </p>
</div>
</blockquote>
<div>
<div class="gmail_default"
style="font-family:arial,sans-serif">I do agree that we
would need a mutex containing both the seek and read for a
block read. Also, on reflection I can see that this could
add a lot of condition as the mutex would be held for very
long periods of time and it would intefere with the
ability to do parallel reads on filesystems where that is
possible (I have to get out of a "file system is a single
spinning disk" mindset). So perhaps I'm not so attached
to the avoiding per-thread file descriptors though I'm not
sure per thread file descriptors would always be the right
answer. <br>
</div>
</div>
</div>
</div>
</blockquote>
<p>One indeed needs to have in mind also the case of /vsicurl/ and
friends where read operations are very slow, so locking isn't
acceptable. That said /vsicurl/ in recent GDAL versions has a
PRead() implementation (which must be used with care, because
currently it doesn't do any caching, and if you ask to read 1
byte, it will issue a HTTP GET Range request for 1 byte, contrary
to standard Read() that reads by chunks of 16 kB and cache them)<br>
</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:CA+YzLBeoC4qx8dSxe8s_TV5if8+AJf_do3qs8Du_LLkRLVSLGg@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<div> </div>
<div>
<div class="gmail_default"
style="font-family:arial,sans-serif">...</div>
<br>
</div>
<blockquote class="gmail_quote"
style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
<p><span class="gmail_default"
style="font-family:arial,sans-serif"></span>libtiff
itself has a lot of state variables, in its core or in
its codecs (e.g. the deflate codec has to maintain a
zlib handle as a state variable if doing partial reads
of a tile/strip). Even getting the location of a TIFF
tile is not thread-safe when defered byteount[]/offset[]
array loading is enabled (which is necessary when
dealing with extremely large remote COGs) since it
involves file accesses. <br>
</p>
</div>
</blockquote>
<div>
<div class="gmail_default"
style="font-family:arial,sans-serif">I can see the point
though it is not clear this can't be addressed, and that
it wouldn't be a useful thing in libtiff.</div>
</div>
</div>
</div>
</blockquote>
<p>Taking a step back, I can't think of any popular image library
which fills a similar role to libtiff (that is a libpng, libjpeg,
openjpeg, libwebp, libjxl, etc etc.) that is thread-safe. For most
of them (libpng, libjpeg, libwebp), partly because they implement
scanline based streamed reading logic, where the PNG/JPEG/etc.
handle holds obviously a mutable state, by API design. libjxl
returns a fully decoded image, but has somehow an incremental
decoding API. openjpeg could be the one that is the closest to
libtiff situation, where one - in theory - could decode different
regions in parallel, but there's mutable state involved underneath
(including a state machine).<br>
</p>
<p>I would expect great (and, for once, justified) resistance &
scepticism from libtiff community members to add thread-safety
into it. A thread-safe TIFF reader would be more a separate
library, or at least a clearly identified separate component
within libtiff with a specific namespace. (*)</p>
<p>Even<br>
</p>
<p>(*) It could be argued that coding it in C wouldn't be the best
idea. I've spotted a TIFF Rust library
(<a class="moz-txt-link-freetext" href="https://github.com/image-rs/image-tiff">https://github.com/image-rs/image-tiff</a>) but it is functionally
limited (at least in terms of supported codecs. didn't check
thourougly) and also associates the TIFF "object" with a file
reader with read+seek capabilities, and thus methods like
read_chunk()
(<a class="moz-txt-link-freetext" href="https://github.com/image-rs/image-tiff/blob/6dc7a266d30291db1e706c8133357931f9e2a053/src/decoder/mod.rs#L1012">https://github.com/image-rs/image-tiff/blob/6dc7a266d30291db1e706c8133357931f9e2a053/src/decoder/mod.rs#L1012</a>)
take a mutable self reference, making it incompatible of
thread-safety. So probably a C (or C++) library written and
designed with thread-safety in mind from the start would probably
the most reasonable option for a GDAL use case.<br>
</p>
--
<pre class="moz-signature" cols="72"><a class="moz-txt-link-freetext" href="http://www.spatialys.com">http://www.spatialys.com</a>
My software is free, but my time generally not.</pre>
</body>
</html>