google / benchmark

A microbenchmark support library
Apache License 2.0
8.59k stars 1.57k forks source link

[FR] Large File Support #1725

Closed oToToT closed 6 months ago

oToToT commented 6 months ago

Is your feature request related to a problem? Please describe. A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

Google benchmark looks to be using libc functions like fseeko, ftello, fopen. (IIUC, std::ifstream may results in using these functions.) On some "small" platforms, it might result in EOVERFLOW when dealing with large files.

Describe the solution you'd like A clear and concise description of what you want to happen. Compiles with these three flags enabled: -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE.

this is the portable/POSIX method for transparently using 64bit types everywhere. for details on each define, see:

http://www.gnu.org/s/libc/manual/html_node/Feature-Test-Macros.html

# _LARGEFILE_SOURCE: enable support for new LFS funcs (ftello/etc...)
# _LARGEFILE64_SOURCE: enable support for 64bit variants (off64_t/fseeko64/etc...)
# _FILE_OFFSET_BITS: default to 64bit variants (off_t is defined as off64_t)

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

N/A

Additional context Add any other context or screenshots about the feature request here.

https://issuetracker.google.com/issues/201531268

oToToT commented 6 months ago

(I can work on this.)

dmah42 commented 6 months ago

have you seen this issue anywhere? ifstream is only used in context of opening sysinfo files ("/proc/cpuinfo" and similar) none of which should be "large". ofstream is used to write out files and i suppose these could become large for larger projects, but maybe not on smaller platforms (as they couldn't run such large projects, i suppose).

vapier commented 6 months ago

LFS isn't just about reading the content of large files. inodes can be 64-bit which means any attempt to stat will fail too. and that can be tricky to reproduce as it's up to the underlying filesystem whether/when it returns an inode # that doesn't fit into 32-bits. enabling LFS costs basically nothing.

i'll also note that in order to support 64-bit time_t, you'll need to enable LFS eventually anyways.

oToToT commented 6 months ago

Thanks @vapier for clearer information and sorry for my initial vague arguments.

IIUC, for 64-bit time_t support (to prevent Y2K38), we only need to add -D_TIME_BITS=64 to the compile flag if LFS is already there. If we want to do so, I can adjust #1726 to include it (and rename it to "Enable Large-file Support + 64-bit time_t").

vapier commented 6 months ago

let's keep the two separate for now. LFS has been around for a long time and should be easy enough. 64-bit time_t is a newer project and still semi-influx.