We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 412

Summary: Windows SDL_RWread slowdown in 1.2.10/11
Product: SDL Reporter: Ryan C. Gordon <icculus>
Component: fileAssignee: Sam Lantinga <slouken>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: normal    
Priority: P1    
Version: 1.2.10   
Hardware: x86   
OS: Windows (All)   
URL: http://lists.libsdl.org/pipermail/sdl-libsdl.org/2007-March/060293.html

Description Ryan C. Gordon 2007-03-08 14:58:45 UTC 
(From SDL mailing list. Discussion thread in URL field. --ryan.)


Date: Thu, 08 Mar 2007 20:14:21 +0100
From: Holger Schemel
To: sdl@libsdl.org
Subject: [SDL] SDL_RWread *much* slower on Windows in 1.2.10/11

Hi!

When upgrading my SDL library to 1.2.11 lately, I noticed an extreme
decrease in startup time of my SDL based game "Rocks'n'Diamonds" on
the Windows platform.

The bottleneck was quickly found to be image loading: When loading PCX
files in my game, this results in "IMG_Load()" being around 6-8 times
slower than with the previously used SDL version (1.2.8)!

After a little profiling, I found the cause being a change in
"SDL_RWFromFile()" to using "win32_file_read()" instead of using
"stdio_read()" for file read access on WIN32 platforms. (I found out
that this change was introduced in SDL 1.2.10.)

As I can see, "win32_file_read()" effectively uses "ReadFile()" from
the Windows API, while "stdio_read()" uses standard ANSI-C "fread()".

This problem was confirmed with the same relative speed difference on
two PCs, one with Windows 2000 and one with Windows XP.

After recompiling the SDL.dll using the ANSI-C file access functions
again (by changing the source code a little bit), everything was fast
again. But there must have been a good reason to change this, so this
probably is not the final solution.

Can anybody of the SDL developers comment on why this change was needed
or what might be the reason for the extreme difference in reading bytes
from a file stream?

Best regards,
		Holger
Comment 1 Ryan C. Gordon 2007-06-02 13:59:00 UTC 
Bumping a bunch of bugs to Priority 1 for consideration for the 1.2.12 release.

--ryan.
Comment 2 Sam Lantinga 2007-07-08 23:21:40 UTC 
Wacky, I'm able to reproduce this here.  The difference isn't 6x on my machine, it's more like 3x, but it's still noticeable.  I'll profile the read pattern and see if there's anything wacky going on.
Comment 3 Sam Lantinga 2007-07-08 23:34:43 UTC 
Holy canoli batman!  The SDL_image PCX loader reads 1 byte at a time!

So the question is, after we fix the PCX loader, do we need to implement buffering on top of the Win32 file I/O?
Comment 4 Ryan C. Gordon 2007-07-08 23:44:48 UTC 
There's actually an option to CreateFile() to explicitly disable buffering, and we don't use it...but on a closer reading of MSDN, I'm wondering if that means "don't save in the kernel's file cache for future reads by any process" vs "don't do a readahead buffer for this file in the current process."

...in the former case, yeah, one byte at a time is going to be slow.

I guess this is a regression, I'm just concerned about adding the extra code to buffer it in 1.2 ... it would be better to fix the PCX loader and maybe add this for 1.3. People in 1.2 that _must_ have it can use the C runtime path instead of the Win32 API, or maybe just do less costly I/O calls than one-byte-at-a-time. 

--ryan.
Comment 5 Sam Lantinga 2007-07-08 23:49:41 UTC 
I just looked at the file format and various PCX loaders.  Byte-at-a-time really is the only way to do it without accidentally reading past something at the end of the image data.

I think we can probably get a pretty good win with a simple 1K read-ahead buffer that we mark dirty after a seek or write.
Comment 6 Sam Lantinga 2007-07-09 21:36:48 UTC 
I added a read-ahead buffer to the Win32 file I/O, and that took care of the problem.

This is fixed in revision 3183.  Thanks!