Skip to content

feat(filesystem): Implement custom buffer options for File::open() #1301

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

Merged
merged 3 commits into from
Jul 27, 2025
Merged

feat(filesystem): Implement custom buffer options for File::open() #1301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f6a595d
[CORE] Implement custom buffer options for LocalFile::open()
xezon Jul 17, 2025
7975e89
Add LINEBUF and FULLBUF modes
xezon Jul 18, 2025
1530d08
Polish comments
xezon Jul 18, 2025
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
4 changes: 2 additions & 2 deletions Core/GameEngine/Include/Common/FileSystem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,13 @@
// Includes
//----------------------------------------------------------------------------

#include "Common/file.h"
#include "Common/STLTypedefs.h"
#include "Common/SubsystemInterface.h"

//----------------------------------------------------------------------------
// Forward References
//----------------------------------------------------------------------------
class File;

//----------------------------------------------------------------------------
// Type Defines
Expand Down Expand Up @@ -130,7 +130,7 @@ class FileSystem : public SubsystemInterface
void reset();
void update();

File* openFile( const Char *filename, Int access = 0 ); ///< opens a File interface to the specified file
File* openFile( const Char *filename, Int access = File::NONE, size_t bufferSize = File::BUFFERSIZE ); ///< opens a File interface to the specified file
Bool doesFileExist(const Char *filename) const; ///< returns TRUE if the file exists. filename should have no directory.
void getFileListInDirectory(const AsciiString& directory, const AsciiString& searchName, FilenameList &filenameList, Bool searchSubdirectories) const; ///< search the given directory for files matching the searchName (egs. *.ini, *.rep). Possibly search subdirectories.
Bool getFileInfo(const AsciiString& filename, FileInfo *fileInfo) const; ///< fills in the FileInfo struct for the file given. returns TRUE if successful.
Expand Down
2 changes: 1 addition & 1 deletion Core/GameEngine/Include/Common/LocalFile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class LocalFile : public File
//virtual ~LocalFile();


virtual Bool open( const Char *filename, Int access = 0 ); ///< Open a file for access
virtual Bool open( const Char *filename, Int access = NONE, size_t bufferSize = BUFFERSIZE ); ///< Open a file for access
virtual void close( void ); ///< Close the file
virtual Int read( void *buffer, Int bytes ); ///< Read the specified number of bytes in to buffer: See File::read
virtual Int write( const void *buffer, Int bytes ); ///< Write the specified number of bytes from the buffer: See File::write
Expand Down
4 changes: 1 addition & 3 deletions Core/GameEngine/Include/Common/LocalFileSystem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@
#include "Common/SubsystemInterface.h"
#include "FileSystem.h" // for typedefs, etc.

class File;

class LocalFileSystem : public SubsystemInterface
{
public:
Expand All @@ -45,7 +43,7 @@ class LocalFileSystem : public SubsystemInterface
virtual void reset() = 0;
virtual void update() = 0;

virtual File * openFile(const Char *filename, Int access = 0) = 0;
virtual File * openFile(const Char *filename, Int access = File::NONE, size_t bufferSize = File::BUFFERSIZE) = 0;
virtual Bool doesFileExist(const Char *filename) const = 0;
virtual void getFileListInDirectory(const AsciiString& currentDirectory, const AsciiString& originalDirectory, const AsciiString& searchName, FilenameList &filenameList, Bool searchSubdirectories) const = 0; ///< search the given directory for files matching the searchName (egs. *.ini, *.rep). Possibly search subdirectories.
virtual Bool getFileInfo(const AsciiString& filename, FileInfo *fileInfo) const = 0; ///< see FileSystem.h
Expand Down
2 changes: 1 addition & 1 deletion Core/GameEngine/Include/Common/RAMFile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class RAMFile : public File
//virtual ~RAMFile();


virtual Bool open( const Char *filename, Int access = 0 ); ///< Open a file for access
virtual Bool open( const Char *filename, Int access = NONE, size_t bufferSize = 0 ); ///< Open a file for access
virtual void close( void ); ///< Close the file
virtual Int read( void *buffer, Int bytes ); ///< Read the specified number of bytes in to buffer: See File::read
virtual Int write( const void *buffer, Int bytes ); ///< Write the specified number of bytes from the buffer: See File::write
Expand Down
2 changes: 1 addition & 1 deletion Core/GameEngine/Include/Common/StreamingArchiveFile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class StreamingArchiveFile : public RAMFile
//virtual ~StreamingArchiveFile();


virtual Bool open( const Char *filename, Int access = 0 ); ///< Open a file for access
virtual Bool open( const Char *filename, Int access = NONE, size_t bufferSize = BUFFERSIZE ); ///< Open a file for access
virtual void close( void ); ///< Close the file
virtual Int read( void *buffer, Int bytes ); ///< Read the specified number of bytes in to buffer: See File::read
virtual Int write( const void *buffer, Int bytes ); ///< Write the specified number of bytes from the buffer: See File::write
Expand Down
30 changes: 23 additions & 7 deletions Core/GameEngine/Include/Common/file.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@
*
* All code should use the File class and not its derivatives, unless
* absolutely necessary. Also FS::Open should be used to create File objects and open files.
*
* TheSuperHackers @feature Adds LINEBUF and FULLBUF modes and buffer size argument for file open.
*/
//===============================

Expand All @@ -89,19 +91,28 @@ class File : public MemoryPoolObject

enum access
{
NONE = 0x00000000,
NONE = 0x00000000, ///< Access file. Reading by default

READ = 0x00000001, ///< Access file for reading
WRITE = 0x00000002, ///< Access file for writing
READWRITE = (READ | WRITE),

APPEND = 0x00000004, ///< Seek to end of file on open
CREATE = 0x00000008, ///< Create file if it does not exist
TRUNCATE = 0x00000010, ///< Delete all data in file when opened

// NOTE: accesses file as text data if neither TEXT and BINARY are set
TEXT = 0x00000020, ///< Access file as text data
BINARY = 0x00000040, ///< Access file as binary data
READWRITE = (READ | WRITE),

ONLYNEW = 0x00000080, ///< Only create file if it does not exist

// NOTE: STREAMING is Mutually exclusive with WRITE
STREAMING = 0x00000100 ///< Do not read this file into a ram file, read it as requested.
STREAMING = 0x00000100, ///< Do not read this file into a ram file, read it as requested.

// NOTE: accesses file with full buffering if neither LINEBUF and FULLBUF are set
LINEBUF = 0x00000200, ///< Access file with line buffering
FULLBUF = 0x00000400, ///< Access file with full buffering
};

enum seekMode
Expand All @@ -111,6 +122,11 @@ class File : public MemoryPoolObject
END ///< Seek position is relative from the end of the file
};

enum
{
BUFFERSIZE = BUFSIZ,
};

protected:

AsciiString m_nameStr; ///< Stores file name
Expand All @@ -128,20 +144,20 @@ class File : public MemoryPoolObject


Bool eof();
virtual Bool open( const Char *filename, Int access = 0 ); ///< Open a file for access
virtual Bool open( const Char *filename, Int access = NONE, size_t bufferSize = BUFFERSIZE ); ///< Open a file for access
virtual void close( void ); ///< Close the file !!! File object no longer valid after this call !!!

virtual Int read( void *buffer, Int bytes ) = NULL ; /**< Read the specified number of bytes from the file in to the
* memory pointed at by buffer. Returns the number of bytes read.
* Returns -1 if an error occured.
* Returns -1 if an error occurred.
*/
virtual Int write( const void *buffer, Int bytes ) = NULL ; /**< Write the specified number of bytes from the
* memory pointed at by buffer to the file. Returns the number of bytes written.
* Returns -1 if an error occured.
* Returns -1 if an error occurred.
*/
virtual Int seek( Int bytes, seekMode mode = CURRENT ) = NULL; /**< Sets the file position of the next read/write operation. Returns the new file
* position as the number of bytes from the start of the file.
* Returns -1 if an error occured.
* Returns -1 if an error occurred.
*
* seekMode determines how the seek is done:
*
Expand Down
4 changes: 2 additions & 2 deletions Core/GameEngine/Source/Common/System/File.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ File::~File()
*/
//=================================================================

Bool File::open( const Char *filename, Int access )
Bool File::open( const Char *filename, Int access, size_t bufferSize )
{
if( m_open )
{
Expand All @@ -160,7 +160,7 @@ Bool File::open( const Char *filename, Int access )
access |= READ;
}

if ( !(access & (READ|APPEND)) )
if ( (access & (READ|APPEND)) == 0 )
{
access |= TRUNCATE;
}
Expand Down
4 changes: 2 additions & 2 deletions Core/GameEngine/Source/Common/System/FileSystem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,14 +171,14 @@ void FileSystem::reset( void )
// FileSystem::open
//============================================================================

File* FileSystem::openFile( const Char *filename, Int access )
File* FileSystem::openFile( const Char *filename, Int access, size_t bufferSize )
{
USE_PERF_TIMER(FileSystem)
File *file = NULL;

if ( TheLocalFileSystem != NULL )
{
file = TheLocalFileSystem->openFile( filename, access );
file = TheLocalFileSystem->openFile( filename, access, bufferSize );

#if ENABLE_FILESYSTEM_EXISTENCE_CACHE
if (file != NULL && (file->getAccess() & File::CREATE))
Expand Down
29 changes: 24 additions & 5 deletions Core/GameEngine/Source/Common/System/LocalFile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,14 +134,14 @@ LocalFile::~LocalFile()
// LocalFile::open
//=================================================================
/**
* This function opens a file using the standard C open() call. Access flags
* are mapped to the appropriate open flags. Returns true if file was opened
* successfully.
* This function opens a file using the standard C open() or
* fopen() call. Access flags are mapped to the appropriate flags.
* Returns true if file was opened successfully.
*/
//=================================================================

//DECLARE_PERF_TIMER(LocalFile)
Bool LocalFile::open( const Char *filename, Int access )
Bool LocalFile::open( const Char *filename, Int access, size_t bufferSize )
{
//USE_PERF_TIMER(LocalFile)
if( !File::open( filename, access) )
Expand Down Expand Up @@ -171,7 +171,6 @@ Bool LocalFile::open( const Char *filename, Int access )

// Mode string selection (mimics _open flag combinations)
// TEXT is implicit for fopen if 'b' is not present
// READ is implicit here if not READWRITE or WRITE
if (readwrite)
{
if (append)
Expand Down Expand Up @@ -199,6 +198,26 @@ Bool LocalFile::open( const Char *filename, Int access )
goto error;
}

{
Int result = 0;

if (bufferSize == 0)
{
result = setvbuf(m_file, NULL, _IONBF, 0); // Uses no buffering
}
else
{
const Int bufferMode = (m_access & LINEBUF)
? _IOLBF // Uses line buffering
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may only want line buffering on text && write ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems sensible to me, since line buffering works by buffering up till the relevant newline or return character is seen.
So you don't want that occurring outside of text mode with binary data.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some thinking I concluded that it is better separated from TEXT and BINARY, so I added new modes LINEBUF and FULLBUF. This way the caller has full control over the behavior.

: _IOFBF; // Uses full buffering

// Buffer is expected to lazy allocate on first read or write later
result = setvbuf(m_file, NULL, bufferMode, bufferSize);
}

DEBUG_ASSERTCRASH(result == 0, ("LocalFile::open - setvbuf failed"));
}

#else

int flags = 0;
Expand Down
15 changes: 9 additions & 6 deletions Core/GameEngine/Source/Common/System/RAMFile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,17 +133,20 @@ RAMFile::~RAMFile()
// RAMFile::open
//=================================================================
/**
* This function opens a file using the standard C open() call. Access flags
* are mapped to the appropriate open flags. Returns true if file was opened
* successfully.
* This function opens a file using the standard C open() or
* fopen() call. Access flags are mapped to the appropriate flags.
* Returns true if file was opened successfully.
*/
//=================================================================

//DECLARE_PERF_TIMER(RAMFile)
Bool RAMFile::open( const Char *filename, Int access )
Bool RAMFile::open( const Char *filename, Int access, size_t bufferSize )
{
//USE_PERF_TIMER(RAMFile)
File *file = TheFileSystem->openFile( filename, access );

bufferSize = 0; // RAM File needs no file buffer because it is read in one go.
Copy link

@Mauller Mauller Jul 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say that it should use the default buffer size as the initial file access will make multiple calls to read individual bytes of data instead of a whole block of 512 bytes at a time from disc etc.

This will be a performance degredation after the previous changes to enable buffering of reads. And RAM files are used frequently within the game, if not they are the most commonly used file type when loading data.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you show where these reads are that you refer to?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just bellow in this same function, when you first use open from the file system, you can either be given a ram file or a local file depending on if the file you want is from local storage or an archive.

The files contents are then read and copied to the calling ram file.

Copy link

@Mauller Mauller Jul 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh wait a sec, it was some github funnyness and i was seeing part of the open( File *file) function bellow as though it was part of this function.

But since the file system can call and load from a local file or archive, that file will be the one affected by the buffersize call rather than the ram file itself.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see problem.


File *file = TheFileSystem->openFile( filename, access, bufferSize );

if ( file == NULL )
{
Expand All @@ -169,7 +172,7 @@ Bool RAMFile::open( File *file )
return NULL;
}

Int access = file->getAccess();
const Int access = file->getAccess();

if ( !File::open( file->getName(), access ))
{
Expand Down
10 changes: 5 additions & 5 deletions Core/GameEngine/Source/Common/System/StreamingArchiveFile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,17 +130,17 @@ StreamingArchiveFile::~StreamingArchiveFile()
// StreamingArchiveFile::open
//=================================================================
/**
* This function opens a file using the standard C open() call. Access flags
* are mapped to the appropriate open flags. Returns true if file was opened
* successfully.
* This function opens a file using the file system. Access flags
* are mapped to the appropriate open flags. Returns true if file
* was opened successfully.
*/
//=================================================================

//DECLARE_PERF_TIMER(StreamingArchiveFile)
Bool StreamingArchiveFile::open( const Char *filename, Int access )
Bool StreamingArchiveFile::open( const Char *filename, Int access, size_t bufferSize )
{
//USE_PERF_TIMER(StreamingArchiveFile)
File *file = TheFileSystem->openFile( filename, access );
File *file = TheFileSystem->openFile( filename, access, bufferSize );

if ( file == NULL )
{
Expand Down
2 changes: 1 addition & 1 deletion Core/GameEngineDevice/Include/StdDevice/Common/StdLocalFileSystem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class StdLocalFileSystem : public LocalFileSystem
virtual void reset();
virtual void update();

virtual File * openFile(const Char *filename, Int access = 0); ///< open the given file.
virtual File * openFile(const Char *filename, Int access = File::NONE, size_t bufferSize = File::BUFFERSIZE); ///< open the given file.
virtual Bool doesFileExist(const Char *filename) const; ///< does the given file exist?

virtual void getFileListInDirectory(const AsciiString& currentDirectory, const AsciiString& originalDirectory, const AsciiString& searchName, FilenameList &filenameList, Bool searchSubdirectories) const; ///< search the given directory for files matching the searchName (egs. *.ini, *.rep). Possibly search subdirectories.
Expand Down
2 changes: 1 addition & 1 deletion Core/GameEngineDevice/Include/Win32Device/Common/Win32LocalFileSystem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class Win32LocalFileSystem : public LocalFileSystem
virtual void reset();
virtual void update();

virtual File * openFile(const Char *filename, Int access = 0); ///< open the given file.
virtual File * openFile(const Char *filename, Int access = File::NONE, size_t bufferSize = File::BUFFERSIZE); ///< open the given file.
virtual Bool doesFileExist(const Char *filename) const; ///< does the given file exist?

virtual void getFileListInDirectory(const AsciiString& currentDirectory, const AsciiString& originalDirectory, const AsciiString& searchName, FilenameList &filenameList, Bool searchSubdirectories) const; ///< search the given directory for files matching the searchName (egs. *.ini, *.rep). Possibly search subdirectories.
Expand Down
3 changes: 2 additions & 1 deletion Core/GameEngineDevice/Source/StdDevice/Common/StdBIGFile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ File* StdBIGFile::openFile( const Char *filename, Int access )
// whoever is opening this file wants write access, so copy the file to the local disk
// and return that file pointer.

File *localFile = TheLocalFileSystem->openFile(filename, access);
CONSTEXPR size_t bufferSize = 0;
File *localFile = TheLocalFileSystem->openFile(filename, access, bufferSize);
if (localFile != NULL) {
ramFile->copyDataToFile(localFile);
}
Expand Down
4 changes: 2 additions & 2 deletions Core/GameEngineDevice/Source/StdDevice/Common/StdLocalFileSystem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ static std::filesystem::path fixFilenameFromWindowsPath(const Char *filename, In
return path;
}

File * StdLocalFileSystem::openFile(const Char *filename, Int access /* = 0 */)
File * StdLocalFileSystem::openFile(const Char *filename, Int access, size_t bufferSize)
{
//USE_PERF_TIMER(StdLocalFileSystem_openFile)

Expand Down Expand Up @@ -155,7 +155,7 @@ File * StdLocalFileSystem::openFile(const Char *filename, Int access /* = 0 */)

StdLocalFile *file = newInstance( StdLocalFile );

if (file->open(path.string().c_str(), access) == FALSE) {
if (file->open(path.string().c_str(), access, bufferSize) == FALSE) {
deleteInstance(file);
file = NULL;
} else {
Expand Down
3 changes: 2 additions & 1 deletion Core/GameEngineDevice/Source/Win32Device/Common/Win32BIGFile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ File* Win32BIGFile::openFile( const Char *filename, Int access )
// whoever is opening this file wants write access, so copy the file to the local disk
// and return that file pointer.

File *localFile = TheLocalFileSystem->openFile(filename, access);
CONSTEXPR size_t bufferSize = 0;
File *localFile = TheLocalFileSystem->openFile(filename, access, bufferSize);
if (localFile != NULL) {
ramFile->copyDataToFile(localFile);
}
Expand Down
4 changes: 2 additions & 2 deletions Core/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ Win32LocalFileSystem::~Win32LocalFileSystem() {
}

//DECLARE_PERF_TIMER(Win32LocalFileSystem_openFile)
File * Win32LocalFileSystem::openFile(const Char *filename, Int access /* = 0 */)
File * Win32LocalFileSystem::openFile(const Char *filename, Int access, size_t bufferSize)
{
//USE_PERF_TIMER(Win32LocalFileSystem_openFile)

Expand Down Expand Up @@ -71,7 +71,7 @@ File * Win32LocalFileSystem::openFile(const Char *filename, Int access /* = 0 */
// TheSuperHackers @fix Mauller 21/04/2025 Create new file handle when necessary to prevent memory leak
Win32LocalFile *file = newInstance( Win32LocalFile );

if (file->open(filename, access) == FALSE) {
if (file->open(filename, access, bufferSize) == FALSE) {
deleteInstance(file);
file = NULL;
} else {
Expand Down
Loading