[GRASS-dev] vector large file support

While I looked into possibilities to optimize v.in.ogr I noticed that grass does not support coor files larger than 2 GB. With topological information stored in that file, and often many dead lines wasting space, the coor file can easily exceed 2 GB nowadays. While v.in.ogr was cleaning one particular vector, the coor file size went up to 9 GB, I killed v.in.ogr before it was finished, the resulting coor file when writing out that vector may have been well above 10 GB. GRASS can process such large coor files (to a degree) as long as topo is kept in memory, e.g. with v.in.ogr and v.clean, and can close such a vector and write it to disk. But that thing can not be opened again, the topo file stores the size of the coor file, and that stored value in the topo file must not exceed 2 GB (integer limit), giving a mismatch and error.

I want to propose some solutions to this problem:
high-level
Modules modifying the coor file, e.g. v.in.ogr, v.clean, v.overlay, v.buffer, should do all the processing in a temporary vector and at the end only copy alive lines to the final output vector, Vect_copy_map_lines() does that. When importing shapefiles with areas I noticed a coor file size reduction by a factor 2 to 5 which is quite a lot (e.g. 1 a GB coor file can be melted down to 200 MB, much nicer). This is also suggested in the vector TODO [1], I'm just pressing again.

low-level
coor file size is stored in memory as type long (32 bit integer on a 32bit system, and on my 64bit Linux with 32bit compatibility) counting bytes of the coor file. That gives the 2 GB limit. When closing the vector, this number is written to the topo file. When opening that vector again, this number is read from the topo file and compared to the actual coor file size, this is the 2 GB limit.
If this coor file size information in the topo file is just a safety check and not needed to process the coor file, it could be omitted altogether, making the supported coor file size unlimited (limited by the current system and filesystem). All references to the coor file size would need to be removed from the vector library.
If the coor file size stored in the topo file is indeed needed to properly process the coor file, the respective variables must be something else than long in order to support coor files larger than 2 GB, maybe long long? Same for all intermediate variables in the vector library storing coor file size.
Looking at limits.h, long can be like int or like long long (only true 64 bit systems). I use Linux 64bit with 32bit compatibility, here long is like int. Someone more familiar with type limits and type declarations on different systems please help!

I suspect some integer overflow for large coor files also in rtree, maybe someone in the know could look into that? :slight_smile:

Regards,

Markus M

[1] http://freegis.org/cgi-bin/viewcvs.cgi/*checkout*/grass6/doc/vector/TODO

Markus Metz wrote:

If the coor file size stored in the topo file is indeed needed to
properly process the coor file, the respective variables must be
something else than long in order to support coor files larger than 2
GB, maybe long long? Same for all intermediate variables in the vector
library storing coor file size.
Looking at limits.h, long can be like int or like long long (only true
64 bit systems). I use Linux 64bit with 32bit compatibility, here long
is like int. Someone more familiar with type limits and type
declarations on different systems please help!

As you note, long will normally be the largest size which the CPU can
handle natively, while long long (only available in C99 or as a gcc
extension) can be expected to be 64 bits where it exists. FWIW, "int"
can theoretically be 64 bits, but this is rare.

The correct type to use for the size of a file is off_t, which can be
made to be a 64-bit type by adding -D_FILE_OFFSET_BITS=64 to the
compilation switches. This should only be done if $(USE_LARGEFILES) is
non-empty (corresponding to --enable-largefile).

However, that alone isn't sufficient, as you have to explicitly force
offset calculations to be performed using off_t rather than int/long,
e.g.:

  long idx, step;
  ...
  off_t offset = (off_t) idx * step;
or:
  off_t offset = idx * (off_t) step;

Note that:

  off_t offset = idx * step;
and:
  off_t offset = (off_t) (idx * step);

won't work, as the result isn't up-cast until after it has been
truncated.

--
Glynn Clements <glynn@gclements.plus.com>

I tried to understand the grass wiki on Large File Support, sorry for being a bit late with that!

Glynn Clements wrote:

Markus Metz wrote:

If the coor file size stored in the topo file is indeed needed to properly process the coor file, the respective variables must be something else than long in order to support coor files larger than 2 GB, maybe long long? Same for all intermediate variables in the vector library storing coor file size.
Looking at limits.h, long can be like int or like long long (only true 64 bit systems). I use Linux 64bit with 32bit compatibility, here long is like int. Someone more familiar with type limits and type declarations on different systems please help!
    
As you note, long will normally be the largest size which the CPU can
handle natively, while long long (only available in C99 or as a gcc
extension) can be expected to be 64 bits where it exists. FWIW, "int"
can theoretically be 64 bits, but this is rare.

The correct type to use for the size of a file is off_t, which can be
made to be a 64-bit type by adding -D_FILE_OFFSET_BITS=64 to the
compilation switches. This should only be done if $(USE_LARGEFILES) is
non-empty (corresponding to --enable-largefile).

However, that alone isn't sufficient, as you have to explicitly force
offset calculations to be performed using off_t rather than int/long,
e.g.:

  long idx, step;
  ...
  off_t offset = (off_t) idx * step;
or:
  off_t offset = idx * (off_t) step;

Note that:

  off_t offset = idx * step;
and:
  off_t offset = (off_t) (idx * step);

won't work, as the result isn't up-cast until after it has been
truncated.
  

I think I understand. So according to the grass wiki the steps to enable large file support would be

1) add
ifneq ($(USE_LARGEFILES),)
EXTRA_CFLAGS = -D_FILE_OFFSET_BITS=64
endif

to all relevant Makefiles

2) use off_t where appropriate, and take care with type casting. file offset is used in various different places in the vector library, a bit of work to get off_t usage right.

3) solve the fseek/fseeko and ftell/ftello problem. Get inspiration from libgis and LFS-safe modules? Or as suggested in the grass wiki on LFS, add
extern off_t G_ftell(FILE *fp);
extern int G_fseek(FILE *stream, off_t offset, int whence);
for global use?

4) figure out if coor file size really needs to be stored in coor and topo. coor file size doesn't say a lot about the number of features because coor can contain a high proportion of dead lines (a problem in itself, vector TODO). If if does not need to be stored in coor and topo, how does removing coor file size info affect reading and writing of coor and topo? Are there hard-coded offsets for reading these files?

It would be great to have LFS support in vector libs in grass7! I am getting coor files > 2GB more and more often with v.in.ogr and v.clean, and I suspect that modifying a coor file > 2GB, even if the module does the work and does not complain, produces unusable results. I can now modify a module so that it takes say a 1GB coor file, works on it, e.g. do some cleaning, coor file grows over 2GB in the process, at the end of the module only alive lines are written out and the resulting coor file is again below 2 GB. But in between some unnoticed errors may have occurred. This is no good.

Regards,

Markus

Markus Metz wrote:

I think I understand. So according to the grass wiki the steps to enable
large file support would be

1) add
ifneq ($(USE_LARGEFILES),)
EXTRA_CFLAGS = -D_FILE_OFFSET_BITS=64
endif

to all relevant Makefiles

Yes. Although this should probably come after 2 and 3 :wink: First, make
the code safe for LFS, *then* enable it.

2) use off_t where appropriate, and take care with type casting. file
offset is used in various different places in the vector library, a bit
of work to get off_t usage right.

Using off_t isn't a problem; it's when you generate an off_t from
smaller types that care needs to be taken.

3) solve the fseek/fseeko and ftell/ftello problem. Get inspiration from
libgis and LFS-safe modules? Or as suggested in the grass wiki on LFS, add
extern off_t G_ftell(FILE *fp);
extern int G_fseek(FILE *stream, off_t offset, int whence);
for global use?

That would be my preference.

4) figure out if coor file size really needs to be stored in coor and
topo. coor file size doesn't say a lot about the number of features
because coor can contain a high proportion of dead lines (a problem in
itself, vector TODO). If if does not need to be stored in coor and topo,
how does removing coor file size info affect reading and writing of coor
and topo? Are there hard-coded offsets for reading these files?

No idea here.

--
Glynn Clements <glynn@gclements.plus.com>

Glynn Clements wrote:

Markus Metz wrote:

I think I understand. So according to the grass wiki the steps to enable large file support would be

1) add
ifneq ($(USE_LARGEFILES),)
EXTRA_CFLAGS = -D_FILE_OFFSET_BITS=64
endif

to all relevant Makefiles
    
Yes. Although this should probably come after 2 and 3 :wink: First, make
the code safe for LFS, *then* enable it.
  

No chronological order intended, I'm relying on your help :slight_smile:

  

2) use off_t where appropriate, and take care with type casting. file offset is used in various different places in the vector library, a bit of work to get off_t usage right.
    
Using off_t isn't a problem; it's when you generate an off_t from
smaller types that care needs to be taken.
  

Yes, that's what I meant, because the vector library uses file offset a lot, all of type long so far. It is doable to update the vector library with off_t usage and proper casting as you pointed out previously, but file offset is an integral part of grass vector topology and must be handled with care, I'm aware of that.

  

3) solve the fseek/fseeko and ftell/ftello problem. Get inspiration from libgis and LFS-safe modules? Or as suggested in the grass wiki on LFS, add
extern off_t G_ftell(FILE *fp);
extern int G_fseek(FILE *stream, off_t offset, int whence);
for global use?
    
That would be my preference.
  

How about

extern off_t G_ftell(FILE *fp)
{
#ifdef HAVE_LARGEFILES
    return (ftello(fp);
#else
    return (ftell(fp);
#endif }

That is the current solution in the iostream library for fseek/fseeko. I know it's not that easy according to the grass wiki on LFS support, "The issues". Apparently just because you configure with --enable-largefile doesn't mean fseeko and ftello are available, although most modern systems should have it. ftello is currently not used in devbr_6 and trunk.

  

4) figure out if coor file size really needs to be stored in coor and topo. coor file size doesn't say a lot about the number of features because coor can contain a high proportion of dead lines (a problem in itself, vector TODO). If if does not need to be stored in coor and topo, how does removing coor file size info affect reading and writing of coor and topo? Are there hard-coded offsets for reading these files?
    
No idea here.
  

I think coor file size is stored as safety check but not needed to read features. That safety check is probably there for a good reason.
The problem is that the offset of a given feature in the coor file is stored in topo and must be properly retrieved, i.e. the number of bytes used to store a given feature offset must match the off_t size of the current library. A topo file written with LFS support will thus be only readable by old libraries or grass compiled without LFS support after rebuilding topology. The other way around, a vector that was written without LFS support can be opened with new LFS-enabled libraries only after rebuilding topology. The off_t size used to write the vector must be stored somewhere in the header info of topo (most important) and coor. If this size does not match the current off_t size, topology must be rebuild.
Letting older libraries know that topology needs to be rebuilt is the easiest part...

Nonsense or going into the right direction?

I'm afraid it will be a long way to get LFS into the vector libraries.

Markus Metz wrote:

>> 3) solve the fseek/fseeko and ftell/ftello problem. Get inspiration from
>> libgis and LFS-safe modules? Or as suggested in the grass wiki on LFS, add
>> extern off_t G_ftell(FILE *fp);
>> extern int G_fseek(FILE *stream, off_t offset, int whence);
>> for global use?
>>
>
> That would be my preference.
>
How about

extern off_t G_ftell(FILE *fp)
{
#ifdef HAVE_LARGEFILES
    return (ftello(fp);
#else
    return (ftell(fp);
#endif
}

Yep, other than the extraneous open parenthesis (2 open, 1 close).

G_fseek() is slightly more tricky, as you need to check for an
out-of-range value rather than silently truncating.

>> 4) figure out if coor file size really needs to be stored in coor and
>> topo. coor file size doesn't say a lot about the number of features
>> because coor can contain a high proportion of dead lines (a problem in
>> itself, vector TODO). If if does not need to be stored in coor and topo,
>> how does removing coor file size info affect reading and writing of coor
>> and topo? Are there hard-coded offsets for reading these files?
>>
>
> No idea here.

I think coor file size is stored as safety check but not needed to read
features. That safety check is probably there for a good reason.
The problem is that the offset of a given feature in the coor file is
stored in topo and must be properly retrieved, i.e. the number of bytes
used to store a given feature offset must match the off_t size of the
current library. A topo file written with LFS support will thus be only
readable by old libraries or grass compiled without LFS support after
rebuilding topology. The other way around, a vector that was written
without LFS support can be opened with new LFS-enabled libraries only
after rebuilding topology. The off_t size used to write the vector must
be stored somewhere in the header info of topo (most important) and
coor. If this size does not match the current off_t size, topology must
be rebuild.
Letting older libraries know that topology needs to be rebuilt is the
easiest part...

Nonsense or going into the right direction?

I think that the code which reads these files needs functions to
read/write off_t values at the size used by the file, not the size
used by the code.

I.e. if the code is built for 64-bit off_t, it should still be able to
directly read/write files using a 32-bit off_t. Code built for 32-bit
off_t should also directly read/write files which use a 64-bit off_t,
subject to the constraint that only 31 bits are non-zero (if you have
a 32-bit off_t, attempting to open a file >=2GiB will fail, as will
attempting to enlarge a file beyond that size).

--
Glynn Clements <glynn@gclements.plus.com>

Glynn Clements wrote:

Markus Metz wrote:

[...]
      

How about

extern off_t G_ftell(FILE *fp)
{
#ifdef HAVE_LARGEFILES
    return (ftello(fp);
#else
    return (ftell(fp);
#endif }
    
Yep, other than the extraneous open parenthesis (2 open, 1 close).
  

Oops, my sloppy writing... What about off_t lseek(int fd, off_t offset, int whence) ? From the GNU C library: "The lseek function is the underlying primitive for the fseek, fseeko, ftell, ftello and rewind functions [...]" lseek is used in libgis and several modules, I didn't see something like the above #ifdef construct. Not an option for the vector libs I assume, because these would need to be largely rewritten when using lseek instead of fseek, read instead of fread and so on (using file descriptor instead of stream pointer throughout). Probably a nonsense idea anyway.

  

[...]
    
I think that the code which reads these files needs functions to
read/write off_t values at the size used by the file, not the size
used by the code.

I.e. if the code is built for 64-bit off_t, it should still be able to
directly read/write files using a 32-bit off_t. Code built for 32-bit
off_t should also directly read/write files which use a 64-bit off_t,
subject to the constraint that only 31 bits are non-zero (if you have
a 32-bit off_t, attempting to open a file >=2GiB will fail, as will
attempting to enlarge a file beyond that size).
  

The problem I see is that offset values are stored in topo and cidx (e.g. the topo file knows that line i is in the coor file at offset o). So if the topo file was written with 64-bit off_t but the current compiled library uses 32-bit off_t, can this 32-bit library somehow get these 64-bit offset values out of the topo file? Granted that these values are in the 32-bit range. I have really no idea if this can be done, my suggestion would be to rebuild topology if there is a mismatch between off_t size used in the topo file and off_t size used by the current library. The other way around may be less problematic, when you have a 64-bit off_t library and a topo file with 32-bit offset values. As long as you know what off_t size was used to write the topo and cidx files. And now the mess starts, I'm afraid. The header of the topo file would need to get modified so that it holds the off_t size used to write this file. This information must be available before any attempt is made to retrieve an offset value from the topo file. Then do some safety checking if the offset values can be properly retrieved, if no, request rebuilding topology...

The functions needed to read/write off_t you mention above will not be as easy as the current functions to read/write e.g. int or long, because the current code uses a fixed length for int and long (and all others) that is independent of the system to guarantee portability. The functions to read/write off_t need to work with variable off_t size, because LFS may not always be requested when compiling grass and because it would be nice if topo files written with an off_t size different from the current library can be retrieved.

I get the impression that the code would have to be changed considerably, that all sorts of safety checks need to be built in, that different off_t sizes must be supported independent of LFS presence, that the layout of the topo file needs to be changed, the layout of the cidx file maybe not maybe yes, and that it will create some forward/backward compatibility problems when reading even small vectors with different grass versions. There were big changes in both the raster and the vector model before, grass7 would be an opportunity to introduce changes, but this is getting a bit too much for me and I think I should leave this to people in the know instead of making wild suggestions.

Markus M

Markus Metz wrote:

What about off_t lseek(int fd, off_t offset, int whence) ?
From the GNU C library: "The lseek function is the
underlying primitive for the fseek, fseeko, ftell, ftello and rewind
functions [...]" lseek is used in libgis and several modules, I didn't
see something like the above #ifdef construct.

lseek() always uses off_t. Originally it used long (hence the name
"l"seek), but that's ancient history; you won't find such a system
outside of a museum.

_FILE_OFFSET_BITS determines whether off_t is 32 or 64 bits. If it's
64 bits, many of the POSIX I/O functions (open, read, write, lseek)
are redirected to 64-bit equivalents (open64, read64, etc).

Not an option for the
vector libs I assume, because these would need to be largely rewritten
when using lseek instead of fseek, read instead of fread and so on
(using file descriptor instead of stream pointer throughout). Probably a
nonsense idea anyway.

It's not worth using "raw" I/O just to avoid this issue. Apart from
anything else, there's a potentially huge performance hit, as the
vector library tends to use many small read/write operations. Using
low-level I/O requires a system call for each operation, while the
stdio interface will coalesce these, reading/writing whole blocks.

> I think that the code which reads these files needs functions to
> read/write off_t values at the size used by the file, not the size
> used by the code.
>
> I.e. if the code is built for 64-bit off_t, it should still be able to
> directly read/write files using a 32-bit off_t. Code built for 32-bit
> off_t should also directly read/write files which use a 64-bit off_t,
> subject to the constraint that only 31 bits are non-zero (if you have
> a 32-bit off_t, attempting to open a file >=2GiB will fail, as will
> attempting to enlarge a file beyond that size).
>
The problem I see is that offset values are stored in topo and cidx
(e.g. the topo file knows that line i is in the coor file at offset o).
So if the topo file was written with 64-bit off_t but the current
compiled library uses 32-bit off_t, can this 32-bit library somehow get
these 64-bit offset values out of the topo file?

In the worst case, it can just perform 2 32-bit reads, and check that
the high word is zero and the low word is positive.

If the topo file contains any offsets which exceed the 2GiB range,
then the coor file will be larger than 2GiB. If you aren't using
_FILE_OFFSET_BITS=64, open()ing the coor file will likely fail.

Granted that these
values are in the 32-bit range. I have really no idea if this can be
done, my suggestion would be to rebuild topology if there is a mismatch
between off_t size used in the topo file and off_t size used by the
current library. The other way around may be less problematic, when you
have a 64-bit off_t library and a topo file with 32-bit offset values.
As long as you know what off_t size was used to write the topo and cidx
files. And now the mess starts, I'm afraid. The header of the topo file
would need to get modified so that it holds the off_t size used to write
this file. This information must be available before any attempt is made
to retrieve an offset value from the topo file. Then do some safety
checking if the offset values can be properly retrieved, if no, request
rebuilding topology...

As an alternative, you could just examine the header size, which
precedes the header. As the header contains offsets, the size will
vary with the offset size.

OTOH, this amounts to a format change, so you may as well just add a
new field to the header. Either way, the version number needs to be
increased.

--
Glynn Clements <glynn@gclements.plus.com>

Glynn Clements wrote:

lseek() always uses off_t. Originally it used long (hence the name
"l"seek), but that's ancient history; you won't find such a system
outside of a museum.

_FILE_OFFSET_BITS determines whether off_t is 32 or 64 bits. If it's
64 bits, many of the POSIX I/O functions (open, read, write, lseek)
are redirected to 64-bit equivalents (open64, read64, etc).
  

That's why I asked if fseek, fread, fwrite etc can be replaced with lseek, read, write etc :slight_smile: , no need to check HAVE_LARGEFILES with lseek etc, just compile with -D_FILE_OFFSET_BITS=64.
Do I understand right that fseeko and ftello are only needed on 32-bit systems that want D_FILE_OFFSET_BITS=64? fseek e.g. returns long which is on my 64bit Linux 64bit, I guess that's why I can write coor files > 2GB with the current vector libs.

It's not worth using "raw" I/O just to avoid this issue. Apart from
anything else, there's a potentially huge performance hit, as the
vector library tends to use many small read/write operations. Using
low-level I/O requires a system call for each operation, while the
stdio interface will coalesce these, reading/writing whole blocks.
  

Interesting and good to know. So we do need G_fseek() and G_ftell()

  

The problem I see is that offset values are stored in topo and cidx (e.g. the topo file knows that line i is in the coor file at offset o). So if the topo file was written with 64-bit off_t but the current compiled library uses 32-bit off_t, can this 32-bit library somehow get these 64-bit offset values out of the topo file?
    
In the worst case, it can just perform 2 32-bit reads, and check that
the high word is zero and the low word is positive.
  

Uff. Some more safety checks in the code. From a coding perspective it's easier just to request a topology rebuild. Annoying for the user though. OTOH, that coor file size check is done before anything is read from the coor file, the libs could say something like "Sorry, that vector is too big for you. Please recompile GRASS with LFS" (more friendly phrasing needed). Also potentially annoying. But if the coor file size check is passed (<= 2GB), the high word must be always zero, otherwise it would refer to an offset beyond EOF. You could just use the low word value. Would you have to swap high word and low word if the byte order of the vector is different from the byte order of the current system? Can happen when e.g. a whole grass location is copied to another system. I think not because the vector libs use their own fixed byte order. I would really just request a topology rebuild to avoid all this hassle.

If the topo file contains any offsets which exceed the 2GiB range,
then the coor file will be larger than 2GiB. If you aren't using
_FILE_OFFSET_BITS=64, open()ing the coor file will likely fail.
  

Opening the coor file is not even attempted with the current code in this situation, because the coor file size stored in the topo header can not be larger than 2GB and this size is used for a safety check before opening the coor file. Actually, I don't know what would happen on a 32-bit system. If new vector libs are compiled without LFS, does a 32-bit system have a chance to find out that the coor file is too large? To be precise, when calling stat(path, &stat_buf), what would be the maximum possible value of stat_buf.st_size in 32-bit? Likely LONG_MAX.

OTOH, this amounts to a format change, so you may as well just add a
new field to the header. Either way, the version number needs to be
increased.
  

Increasing the minor version number of topo should be sufficient, but the backwards compatibility minor version number of topo must also be increased to enforce rebuilding of topology when vectors written with new libs are opened with old libs, that will write new topo and cidx files. I would try to keep the coor version numbers as they are, that would at least give backwards/forwards portability of vector files. cidx version numbers could stay unchanged, only that offset values could be stored as 64bit. But topo is read first, the information in the header of the topo file can (must?) be used for safety checks. I guess we are lost if someone produces a topo file >2GB, but a vector with such a large topo file would be a nightmare to work with anyway. No idea if this still holds true in say 5 years from now (I got max 600MB already, unworkable though because no LFS in vector libs and coor >2GB).

I think we could soon come up with a detailed plan of action: what are the currently known caveats, what should be done where in what order to get LFS into the vector libs. Anybody taking on this task would profit from such a guideline, with a big warning that the suggested changes may not be sufficient, that something may have been missed, and that the list of caveats is most likely not complete.

Lots of "if"s and "but"s and "?" in this post of mine.

PS: Thanks for your patience, Glynn.

Markus Metz wrote:

Do I understand right that fseeko and ftello are only needed on 32-bit
systems that want D_FILE_OFFSET_BITS=64? fseek e.g. returns long which
is on my 64bit Linux 64bit, I guess that's why I can write coor files >
2GB with the current vector libs.

Yes. There's no point in using them unless off_t is larger than long
(i.e. 64-bit off_t versus 32-bit long).

> It's not worth using "raw" I/O just to avoid this issue. Apart from
> anything else, there's a potentially huge performance hit, as the
> vector library tends to use many small read/write operations. Using
> low-level I/O requires a system call for each operation, while the
> stdio interface will coalesce these, reading/writing whole blocks.

Interesting and good to know. So we do need G_fseek() and G_ftell()

Yes. Those would be useful regardless of anything related to the
vector format.

>> The problem I see is that offset values are stored in topo and cidx
>> (e.g. the topo file knows that line i is in the coor file at offset o).
>> So if the topo file was written with 64-bit off_t but the current
>> compiled library uses 32-bit off_t, can this 32-bit library somehow get
>> these 64-bit offset values out of the topo file?
>>
>
> In the worst case, it can just perform 2 32-bit reads, and check that
> the high word is zero and the low word is positive.

Uff. Some more safety checks in the code. From a coding perspective it's
easier just to request a topology rebuild. Annoying for the user though.
OTOH, that coor file size check is done before anything is read from the
coor file, the libs could say something like "Sorry, that vector is too
big for you. Please recompile GRASS with LFS" (more friendly phrasing
needed). Also potentially annoying.

Right. But if you have a >=2GiB coor file with a 32-bit off_t, the OS
will refuse to open() to the coor file regardless of any checks GRASS
performs.

But if the coor file size check is
passed (<= 2GB), the high word must be always zero, otherwise it would
refer to an offset beyond EOF. You could just use the low word value.
Would you have to swap high word and low word if the byte order of the
vector is different from the byte order of the current system?

Yes.

Can
happen when e.g. a whole grass location is copied to another system. I
think not because the vector libs use their own fixed byte order. I
would really just request a topology rebuild to avoid all this hassle.

Bear in mind that a GRASS database may be on a networked file system,
and accessed by both 32- and 64-bit systems, and by both big- and
little-endian systems.

Also, the user shouldn't need write permission in order to read a map.
Or, rather, don't assume that the user has write permission for a map
which they are reading.

> If the topo file contains any offsets which exceed the 2GiB range,
> then the coor file will be larger than 2GiB. If you aren't using
> _FILE_OFFSET_BITS=64, open()ing the coor file will likely fail.

Opening the coor file is not even attempted with the current code in
this situation, because the coor file size stored in the topo header can
not be larger than 2GB and this size is used for a safety check before
opening the coor file. Actually, I don't know what would happen on a
32-bit system. If new vector libs are compiled without LFS, does a
32-bit system have a chance to find out that the coor file is too large?
To be precise, when calling stat(path, &stat_buf), what would be the
maximum possible value of stat_buf.st_size in 32-bit? Likely LONG_MAX.

Effectively; using stat() on a file >=2GiB results in:

       EOVERFLOW
        (stat()) path refers to a file whose size cannot be represented
        in the type off_t. This can occur when an application
        compiled on a 32-bit platform without -D_FILE_OFFSET_BITS=64
        calls stat() on a file whose size exceeds (2<<31)-1 bits.

--
Glynn Clements <glynn@gclements.plus.com>

Glynn Clements <glynn@gclements.plus.com> writes:

>> Do I understand right that fseeko and ftello are only needed on
>> 32-bit systems that want D_FILE_OFFSET_BITS=64? fseek e.g. returns
>> long which is on my 64bit Linux 64bit, I guess that's why I can
>> write coor files > 2GB with the current vector libs.

> Yes. There's no point in using them unless off_t is larger than long
> (i.e. 64-bit off_t versus 32-bit long).

  I think there is. Per POSIX, off_t is /the/ type to hold file
  offsets and lengths. The intention is for the program to retain
  portability whatever numeric types exists on a target. I guess,
  it will even work if none of the standard C `int' types match
  `off_t' (although it's probably quite unlikely to become an
  issue for GRASS in a foreseeable future.)

>>> It's not worth using "raw" I/O just to avoid this issue. Apart from
>>> anything else, there's a potentially huge performance hit, as the
>>> vector library tends to use many small read/write operations. Using
>>> low-level I/O requires a system call for each operation, while the
>>> stdio interface will coalesce these, reading/writing whole blocks.

>> Interesting and good to know. So we do need G_fseek() and G_ftell()

> Yes. Those would be useful regardless of anything related to the
> vector format.

  And, as it seems, Gnulib already has them:

http://www.gnu.org/software/gnulib/MODULES.html#module=ftello
http://www.gnu.org/software/gnulib/MODULES.html#module=fseeko

[...]

PS. ... And please forgive me for mentioning Gnulib with such a
  persistence without investing a bit of time to further its
  adoption into GRASS. Unfortunately, I'm somewhat overloaded
  with my work lately.

Glynn Clements wrote:

Markus Metz wrote:

Do I understand right that fseeko and ftello are only needed on 32-bit systems that want D_FILE_OFFSET_BITS=64? fseek e.g. returns long which is on my 64bit Linux 64bit, I guess that's why I can write coor files > 2GB with the current vector libs.
    
Yes. There's no point in using them unless off_t is larger than long
(i.e. 64-bit off_t versus 32-bit long).
  

I like the point of Ivan that off_t is the native type for file offsets. Could G_fseek then use fseeko whenever fseeko is available (ditto for ftello)?

  

So we do need G_fseek() and G_ftell()
    
Yes. Those would be useful regardless of anything related to the
vector format.
  

So it's about time these functions get implemented :wink:

Bear in mind that a GRASS database may be on a networked file system,
and accessed by both 32- and 64-bit systems, and by both big- and
little-endian systems.

Also, the user shouldn't need write permission in order to read a map. Or, rather, don't assume that the user has write permission for a map
which they are reading.
  

OK, the biggest problem is to support reading a vector written with sizeof(off_t) == 8 when the libs use sizeof(off_t) == 4, without rebuilding topology. As you suggested, 2 32bit reads can be done, and depending on the endian-ness of the host system either the high word value or the low word value used. To read and write offsets, two new functions are needed anyway in diglib/portable.c, something like dig__fread_port_O() and dig__fwrite_port_O(). Type size mismatch and/or endian-ness mismatch is already handled by the current code for other types. In this particular case reading offset twice seems less of a hassle than I thought first (recycle the argument for the number of reads as used by dig__fread_port_L(), reading long).
If a vector was written with sizeof(off_t) == 4 but the libs use sizeof(off_t) == 8, the handling for reading could be the same like currently done for long.
When writing offsets, it would be easiest (also safest?) to always use sizeof(off_t) of the libs. There will be no mix of different offset sizes because topo and cidx are currently written anew when the vector was updated.

Markus Metz wrote:

>> Do I understand right that fseeko and ftello are only needed on 32-bit
>> systems that want D_FILE_OFFSET_BITS=64? fseek e.g. returns long which
>> is on my 64bit Linux 64bit, I guess that's why I can write coor files >
>> 2GB with the current vector libs.
>>
>
> Yes. There's no point in using them unless off_t is larger than long
> (i.e. 64-bit off_t versus 32-bit long).

I like the point of Ivan that off_t is the native type for file offsets.
Could G_fseek then use fseeko whenever fseeko is available (ditto for
ftello)?

Well, that's the general idea. The only advantage of fseek/ftell is
that they are always available.

> Bear in mind that a GRASS database may be on a networked file system,
> and accessed by both 32- and 64-bit systems, and by both big- and
> little-endian systems.
>
> Also, the user shouldn't need write permission in order to read a map.
> Or, rather, don't assume that the user has write permission for a map
> which they are reading.

OK, the biggest problem is to support reading a vector written with
sizeof(off_t) == 8 when the libs use sizeof(off_t) == 4, without
rebuilding topology.

The biggest problem is when the compiler doesn't provide a 64-bit
integral type (off_t doesn't necessarily have to be 64 bits).

As you suggested, 2 32bit reads can be done, and
depending on the endian-ness of the host system either the high word
value or the low word value used.

The low word is always used. That might be the first word or the
second word, but it's always the low word.

To read and write offsets, two new
functions are needed anyway in diglib/portable.c, something like
dig__fread_port_O() and dig__fwrite_port_O().

Yep.

Type size mismatch and/or
endian-ness mismatch is already handled by the current code for other
types. In this particular case reading offset twice seems less of a
hassle than I thought first (recycle the argument for the number of
reads as used by dig__fread_port_L(), reading long).
If a vector was written with sizeof(off_t) == 4 but the libs use
sizeof(off_t) == 8, the handling for reading could be the same like
currently done for long.
When writing offsets, it would be easiest (also safest?) to always use
sizeof(off_t) of the libs. There will be no mix of different offset
sizes because topo and cidx are currently written anew when the vector
was updated.

It would be both easiest and safest. Although it would be preferable
to use 32 bits if that is known to be sufficient, I don't know whether
this is feasible.

I tried to do the same thing with the raster row offsets, but you
can't tell whether you need 32 or 64 bits until it's too late, so it
always uses 64 bits.

--
Glynn Clements <glynn@gclements.plus.com>

Glynn Clements wrote:

Markus Metz wrote:
  

I like the point of Ivan that off_t is the native type for file offsets. Could G_fseek then use fseeko whenever fseeko is available (ditto for ftello)?
    
Well, that's the general idea. The only advantage of fseek/ftell is
that they are always available.
  

I think grass7 should get LFS support as far as possible, today's datasets can easily exceed the 2GB limit. Before modules can get LFS, the underlying libraries must be enabled. According to the LFS wish list in the wiki on LFS, these are the vector libs and the DB libs. For that, this fseeko/ftello problem needs to be solved for 32bit systems. I have read "The issues" and understand the problem, but some sort of implementation of G_fseek and G_ftell is needed, otherwise modules and libraries need a workaround like the iostream library is doing now. Instead of having many (potentially different) workarounds, one proper solution is preferable. This may not be easy, and as much as I like tackling not easy problems, here I can only say: Please do it!.

  

Bear in mind that a GRASS database may be on a networked file system,
and accessed by both 32- and 64-bit systems, and by both big- and
little-endian systems.

Also, the user shouldn't need write permission in order to read a map. Or, rather, don't assume that the user has write permission for a map
which they are reading.
      

OK, the biggest problem is to support reading a vector written with sizeof(off_t) == 8 when the libs use sizeof(off_t) == 4, without rebuilding topology.
    
The biggest problem is when the compiler doesn't provide a 64-bit
integral type (off_t doesn't necessarily have to be 64 bits).
  

There is a handy function called buf_alloc() in the vector libs, allocating a temporary buffer of the needed size (can be of any size), to read content of any of the vector files. You could then read this temporary buffer in chunks of the size supported by the current vector libs. The code is essentially there and would need only little adjustment.

  

As you suggested, 2 32bit reads can be done, and depending on the endian-ness of the host system either the high word value or the low word value used.
    
The low word is always used. That might be the first word or the
second word, but it's always the low word.
  

I got confused by this endian-ness and confused low/high word with first/second word. With the current code, the low word would be the second word when doing 2 32bit reads on a 64bit sized buffer, independent on a endian-ness mismatch. In this case, the libs would have to check if the high word is != 0 and then exit with an ERROR message, right?

When writing offsets, it would be easiest (also safest?) to always use sizeof(off_t) of the libs. There will be no mix of different offset sizes because topo and cidx are currently written anew when the vector was updated.
    
It would be both easiest and safest. Although it would be preferable
to use 32 bits if that is known to be sufficient, I don't know whether
this is feasible.
  

I don't think so. With v.in.ogr, you have no chance to estimate the coor file size. Coming back to my test shapefile for v.in.ogr with a total size below 5MB, that thing results in a coor file > 8GB with cleaning and > 4GB without cleaning. When working on a grass vector, each module would have to estimate the increase of the coor file. Most modules copy the input vector to the output vector, do the requested modifications on the output vector and write out the output vector. You would have to do some very educated guessing on the size of the final coor file, considering the expected amount of dead lines and the expected amount of additional vertices, to decide if a 32bit off_t would be sufficient. Instead I would prefer to use 64 bits whenever possible. Personally, I would regard 32bit support as a courtesy, but please don't start a discussion about that.

Markus Metz wrote:

I have
read "The issues" and understand the problem, but some sort of
implementation of G_fseek and G_ftell is needed, otherwise modules and
libraries need a workaround like the iostream library is doing now.
Instead of having many (potentially different) workarounds, one proper
solution is preferable. This may not be easy, and as much as I like
tackling not easy problems, here I can only say: Please do it!.

I have added G_fseek() and G_ftell() to 7.0 in r35818.

>> As you suggested, 2 32bit reads can be done, and
>> depending on the endian-ness of the host system either the high word
>> value or the low word value used.
>>
>
> The low word is always used. That might be the first word or the
> second word, but it's always the low word.

I got confused by this endian-ness and confused low/high word with
first/second word. With the current code, the low word would be the
second word when doing 2 32bit reads on a 64bit sized buffer,
independent on a endian-ness mismatch. In this case, the libs would have
to check if the high word is != 0 and then exit with an ERROR message,
right?

Right. The files are always written big-endian, so the high word will
always be first in the file.

As well as checking that the high word is zero, you also need to check
that the low word is <= 0x7fffffff (off_t is signed, hence the limit
being 2GiB not 4GiB).

>> When writing offsets, it would be easiest (also safest?) to always use
>> sizeof(off_t) of the libs. There will be no mix of different offset
>> sizes because topo and cidx are currently written anew when the vector
>> was updated.
>>
>
> It would be both easiest and safest. Although it would be preferable
> to use 32 bits if that is known to be sufficient, I don't know whether
> this is feasible.

I don't think so. With v.in.ogr, you have no chance to estimate the coor
file size. Coming back to my test shapefile for v.in.ogr with a total
size below 5MB, that thing results in a coor file > 8GB with cleaning
and > 4GB without cleaning. When working on a grass vector, each module
would have to estimate the increase of the coor file. Most modules copy
the input vector to the output vector, do the requested modifications on
the output vector and write out the output vector. You would have to do
some very educated guessing on the size of the final coor file,
considering the expected amount of dead lines and the expected amount of
additional vertices, to decide if a 32bit off_t would be sufficient.
Instead I would prefer to use 64 bits whenever possible. Personally, I
would regard 32bit support as a courtesy, but please don't start a
discussion about that.

The issue is whether the coor file size is known at the point that you
start writing the topo/cidx files. If the files are generated
concurrently, then it isn't feasible. If the coor file is generated
first, then it is.

--
Glynn Clements <glynn@gclements.plus.com>

Glynn Clements wrote:

I have added G_fseek() and G_ftell() to 7.0 in r35818.
  

:slight_smile:

  

I got confused by this endian-ness and confused low/high word with first/second word. With the current code, the low word would be the second word when doing 2 32bit reads on a 64bit sized buffer, independent on a endian-ness mismatch. In this case, the libs would have to check if the high word is != 0 and then exit with an ERROR message, right?
    
Right. The files are always written big-endian, so the high word will
always be first in the file.
  

I'm not so sure about that, why is byte order stored in the topo header? Byte order for writing out is determined just before writing topo/cidx.

As well as checking that the high word is zero, you also need to check
that the low word is <= 0x7fffffff (off_t is signed, hence the limit
being 2GiB not 4GiB).
  

OK. Additionally the whole thing should not be negative, that would be an invalid offset.

The issue is whether the coor file size is known at the point that you
start writing the topo/cidx files. If the files are generated
concurrently, then it isn't feasible. If the coor file is generated
first, then it is.
  

You're right, the coor file is generated first. Since we will have a flexible off_t size for the topo and cidx file, we can set it just before writing out these files.

Markus Metz wrote:

>> I got confused by this endian-ness and confused low/high word with
>> first/second word. With the current code, the low word would be the
>> second word when doing 2 32bit reads on a 64bit sized buffer,
>> independent on a endian-ness mismatch. In this case, the libs would have
>> to check if the high word is != 0 and then exit with an ERROR message,
>> right?
>>
>
> Right. The files are always written big-endian, so the high word will
> always be first in the file.

I'm not so sure about that, why is byte order stored in the topo header?
Byte order for writing out is determined just before writing topo/cidx.

Okay; maybe that's just the internal default. I'm looking at
dig_init_portable(), which copies the *_cnvrt arrays for big-endian
and reverses them for little-endian.

I don't know why that code needs to be so obtuse.

> As well as checking that the high word is zero, you also need to check
> that the low word is <= 0x7fffffff (off_t is signed, hence the limit
> being 2GiB not 4GiB).

OK. Additionally the whole thing should not be negative, that would be
an invalid offset.

I'm assuming that the individual words are treated as unsigned.

--
Glynn Clements <glynn@gclements.plus.com>

Glynn Clements wrote:

  

Right. The files are always written big-endian, so the high word will
always be first in the file.
      

I'm not so sure about that, why is byte order stored in the topo header? Byte order for writing out is determined just before writing topo/cidx.
    
Okay; maybe that's just the internal default. I'm looking at
dig_init_portable(), which copies the *_cnvrt arrays for big-endian
and reverses them for little-endian.
  

I noticed that too, after my reply. Sometimes I can't keep up with reading that code.

I don't know why that code needs to be so obtuse.
  

That vector lib code has some potential for cleaning up...

  

As well as checking that the high word is zero, you also need to check
that the low word is <= 0x7fffffff (off_t is signed, hence the limit
being 2GiB not 4GiB).
      

OK. Additionally the whole thing should not be negative, that would be an invalid offset.
    
I'm assuming that the individual words are treated as unsigned.
  

Glynn, your idea to set off_t size according to the size of the coor file was *absolutely brillant*!!!
That means that offset values will be stored as 64bit only if the coor file is larger than 2GB and only if the native off_t size supports that. That also means that the topo/cidx files can stay 100% identical to the current version if the coor file is < 2GB. Otherwise they get changed (larger headers), but then the current libs don't have a chance to read this large vector anyway, exiting with an error message. That also means we don't have to make a plan on how to read 64bit offset values when the libs support only 32bit values, because this indicates a large coor file that is unreadable with a 32bit-off_t lib anyway.

I have implemented dig__fread_port_O() and dig__fwrite_port_O() with variable off_t size, added the appropriate tests to diglib/test.c and the tests are passed (only that test.tmp is now larger than test.ok). It is easy to determine the appropriate off_t size and adjust the topo/cidx headers accordingly.

G_fseek and G_ftell also work, thanks!

BTW, display of grass7 with cairo appears to be half the speed of grass65, I think it was faster last week (can't remember the revision). Not sure if this justifies a complaint.

Markus Metz wrote:

BTW, display of grass7 with cairo appears to be half the speed of
grass65, I think it was faster last week (can't remember the revision).
Not sure if this justifies a complaint.

Almost everything related to display has changed between 6.x and 7.0.

One relatively recent change that may cause a slow-down is that the
display library no longer tries to automatically simplify vectors
(reduce consecutive short line segments with a single segment). This
could have a significant impact when viewing detailed vectors at low
zoom.

The main problem with simplification is that it can result in
artifacts which were hidden by the rendering inaccuries in 6.x but are
visible in 7.0

--
Glynn Clements <glynn@gclements.plus.com>

Glynn Clements wrote:

Markus Metz wrote:

BTW, display of grass7 with cairo appears to be half the speed of grass65, I think it was faster last week (can't remember the revision). Not sure if this justifies a complaint.
    
Almost everything related to display has changed between 6.x and 7.0.

One relatively recent change that may cause a slow-down is that the
display library no longer tries to automatically simplify vectors
(reduce consecutive short line segments with a single segment). This
could have a significant impact when viewing detailed vectors at low
zoom.
  

OK. It does have a significant impact when viewing detailed vectors at low zoom (relatively detailed: 25MB coor, 31MB topo, not that large but complicated topology). I can't see a speed difference with small vectors. That just got me by surprise because I was used to a 7.0 display that's faster, better, nicer than the 6.x display. Usually, the screen resolution can't represent that detail (same for raster map display), but I guess it is more correct to try to display all detail.

BTW, I'm testing LFS enabled vector libs in 7.0. So far it compiles as good or bad as before on Linux 32bit with and without --enable-largefile and on Linux 64bit with --enable-largefile. The test in lib/vector/diglib is not passed when compiling on Linux64bit without --enable-largefile, but the libs work anyway. I did not have to increase any of the version numbers, the vectors are fully backwards compatible, tested with 6.4.0RC3. It would be safer to change that though, and change the layout of the topo and cidx file.

IMHO LFS support for vector libs is not necessarily good, currently. In order to manipulate large vectors, a very fast CPU, fast hard drives and lots of RAM is needed, or a lot of patience, because it can take days or weeks to run modules with large vectors as input. That speed issue may change in the future, and I would regard LFS support in the vector libs as an investment in the future. Maybe I should rather try to fix bugs than to add new features...