-
Notifications
You must be signed in to change notification settings - Fork 86
[CORE] 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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you show where these reads are that you refer to? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ) | ||
{ | ||
|
@@ -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 )) | ||
{ | ||
|
There was a problem hiding this comment.
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
?There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.