[ToME 2] Getting serious about the SDL interface (again)

[AnonymousHero]

I'll second Derakon's advice. Performance problems often turn up in really surprising places, so it's rarely useful to try to guess. (Of course, with experience, you'll start to recognize certain "patterns" as being slow, but that's really something you can only rely on when you have lots of experience... You should also still profile in those cases .)

You'll need to set up profiling in the CMakeLists.txt file. You can find a snippet here (not sure if it'll still work, but...):

http://www.cmake.org/pipermail/cmake...ch/013176.html

[Therem Harth]

Thanks. Actually had to add the -pg flag and compile with no optimization (which funnily enough did not worsen the performance).

Anyway the biggest time-eaters are

  %   cumulative   self              self     total           
 time   seconds   seconds    calls   s/call   s/call  name    
 12.58      0.19     0.19      999     0.00     0.00  update_view
 11.92      0.37     0.18    28140     0.00     0.00  process_world
 10.60      0.53     0.16  3052871     0.00     0.00  Rand_div
  5.96      0.62     0.09  1684440     0.00     0.00  perturb_point_end
  5.96      0.71     0.09  1159544     0.00     0.00  Term_queue_char
  5.30      0.79     0.08  1156221     0.00     0.00  map_info
  3.31      0.84     0.05   146608     0.00     0.00  drawTermStuff
  3.31      0.89     0.05   146118     0.00     0.00  Term_text_sdl
  2.65      0.93     0.04  1261836     0.00     0.00  cave_set_feat
  2.65      0.97     0.04   237207     0.00     0.00  luaH_getstr
  2.65      1.01     0.04    36569     0.00     0.00  vprocess_hooks_return
  2.65      1.05     0.04    16673     0.00     0.00  Term_fresh_row_text

update_view and process_world make sense as being inefficient; LoS calculation is probably slow, and process_world is just enormous and handles lots of stuff. Same with generating pseudorandom numbers, from what I've read. drawTermStuff doesn't seem to be that big of a time hog, unless I'm reading things completely wrong.

[AnonymousHero]

Are you running the "maximal stress" scenario, e.g. scrolling with a player-centered view?

Also, I believe the profiler uses sampling, so you'll probably want to run for a little while to get a more reliable picture.

(I'm not entirely sure if the profiler only accounts for the CPU time of the process itself or if "wait time" is counted. I certainly see large CPU spikes for the X11 process when scrolling (when using "compositing") and it would surprise me if this didn't account for the vast majority of the latency observed by the user... but then profiling can lead to surprises)

EDIT: Actually if you add up the UI stuff from that profile, you get quite a considerable proportion of the total time: 12.58% + 5.96% + 3.31% + 3.31% + 2.65% = 28%. It looks like update_view is hideously large... maybe a little refactor of the three steps into individual functions would give a better idea of what, exactly, is so slow in update_view(). I'd expect it to be "Step 3", i.e. the bit where it calls "lite_spot" to actually refresh the UI.

[Therem Harth]

Sorry about the absence, been slightly distracted lately. I'll see what I can do about the SDL code, but don't expect results fast (or necessarily at all).

Also I've been trying to get ToME 2 to compile on Windows 7. It is an epic struggle, and so far one without success; CMake keeps complaining that GCC is not in my PATH, even though it most assuredly is (and the rest of MSys is not). Between that and Git being slow as a beached whale on Windows, I am almost ready to give up.

Not sure what I'd suggest at this point, as ditching the build system entirely seems like a bad idea. Maybe Windows versions should be cross-compiled on Linux and tested under Wine, as originally advised (IIRC by Magnate?). Also, testing the SDL interface under Windows and/or Wine might not be worth the bother.

[AnonymousHero]

Originally Posted by Therem Harth

Sorry about the absence, been slightly distracted lately. I'll see what I can do about the SDL code, but don't expect results fast (or necessarily at all).

Also I've been trying to get ToME 2 to compile on Windows 7. It is an epic struggle, and so far one without success; CMake keeps complaining that GCC is not in my PATH, even though it most assuredly is (and the rest of MSys is not). Between that and Git being slow as a beached whale on Windows, I am almost ready to give up.

Did you run it exactly like this?

$ cmake -G "MinGW Makefiles"
$ mingw32-make

Originally Posted by Therem Harth

Not sure what I'd suggest at this point, as ditching the build system entirely seems like a bad idea. Maybe Windows versions should be cross-compiled on Linux and tested under Wine, as originally advised (IIRC by Magnate?). Also, testing the SDL interface under Windows and/or Wine might not be worth the bother.

I have considered "waf" recently while experimenting a little bit with D (which was a pain in CMake), but that requires Python -- though it's less dependent on the user actually having anything else installed.

Suggestions for build systems are most welcome.

[Therem Harth]

Originally Posted by AnonymousHero

Did you run it exactly like this?

$ cmake -G "MinGW Makefiles"
$ mingw32-make

No, I must have forgotten about that. Thanks, I'll see what I can do.

I have considered "waf" recently while experimenting a little bit with D (which was a pain in CMake), but that requires Python -- though it's less dependent on the user actually having anything else installed.

Suggestions for build systems are most welcome.

I'll be on the lookout for possibly better ones. Alas this is not something I'm very familiar with.

[Therem Harth]

Aha... It had to be

cmake -G "MSYS Makefiles"

Anyway the unlua branch is not compiling. I get

[ 99%] Building C object src/CMakeFiles/tome.dir/main-win.c.obj
d:/Projects/tome2/src/main-win.c:663: warning: 'extract_file_name' defined but not used
[100%] Building C object src/CMakeFiles/tome.dir/readdib.c.obj
d:/Projects/tome2/src/readdib.c: In function 'ReadDIB':
d:/Projects/tome2/src/readdib.c:319: warning: passing argument 3 of 'MakeBitmapAndPalette' from incompatible pointer type
d:/Projects/tome2/src/readdib.c:138: note: expected 'struct HPALETTE__ **' but argument is of type 'void **'
d:/Projects/tome2/src/readdib.c:319: warning: passing argument 4 of 'MakeBitmapAndPalette' from incompatible pointer type
d:/Projects/tome2/src/readdib.c:138: note: expected 'struct HBITMAP__ **' but argument is of type 'void **'
Linking C executable tome.exe
CMakeFiles/tome.dir/objects.a(modules.c.obj):modules.c:(.text+0x68): undefined reference to `private_check_user_directory'
collect2: ld returned 1 exit status
mingw32-make[2]: *** [src/tome.exe] Error 1
mingw32-make[1]: *** [src/CMakeFiles/tome.dir/all] Error 2
mingw32-make: *** [all] Error 2

Also, there's a warning about a Varda spell:

d:/Projects/tome2/src/spells3.c: In function 'varda_light_of_valinor_info':
d:/Projects/tome2/src/spells3.c:4755: warning: format '%ld' expects type 'long int', but argument 4 has type 'int'

[AnonymousHero]

This is the "unlua" branch? If so, I'll just say right now that not necessarily in a very usable state at any given time -- I pretty much only do minimal checking before committing (sometimes not even that unless I'm doing something could result in hard-to-find errors). Testing is appreciated, though!

The error about private_check_user_directory() seems to be since the preprocessor symbol "WINDOWS" is defined -- this means that private_check_user_directory() from main.c doesn't get compiled. This is the case all the way back to 2.3.5.

But I see that the call to private_check_user_directory() from src/modules.c was surrounded with an '#ifdef PRIVATE_USER_PATH' / #end pair prior to git commit c70d7a8c ...

Perhaps this was necessary after all for Windows. Can you please try switching to master and doing a "git revert c70d7a8c" to see if that fixes compilation?

EDIT: Format string warning fixed; I'm not quite ready to push that (I have a few queued up bits that aren't quite finished.)

[Therem Harth]

Git refuses to revert c70d7a8c, it looks like that change hasn't been made in master (yet?). I think.

Anyway attempting to compile master spits out another pile of errors.

d:/Projects/tome2/src/main.c:308: error: expected identifier or '(' before '>>' token
d:/Projects/tome2/src/main.c:313: error: expected '=', ',', ';', 'asm' or '__attribute__' before '&&' token
d:/Projects/tome2/src/main.c:313: error: expected '=', ',', ';', 'asm' or '__attribute__' before '++' token
d:/Projects/tome2/src/main.c:540: error: expected identifier or '(' before 'if'
d:/Projects/tome2/src/main.c:548: error: expected declaration specifiers or '...' before numeric constant
d:/Projects/tome2/src/main.c:548: warning: data definition has no type or storage class
d:/Projects/tome2/src/main.c:548: warning: type defaults to 'int' in declaration of 'process_player_name'
d:/Projects/tome2/src/main.c:548: error: conflicting types for 'process_player_name'
d:/Projects/tome2/src/externs.h:890: note: previous declaration of 'process_player_name' was here
d:/Projects/tome2/src/main.c:552: warning: data definition has no type or storage class
d:/Projects/tome2/src/main.c:552: warning: type defaults to 'int' in declaration of 'quit_aux'
d:/Projects/tome2/src/main.c:552: error: conflicting types for 'quit_aux'
d:/Projects/tome2/src/z-util.h:51: note: previous declaration of 'quit_aux' was here
d:/Projects/tome2/src/main.c:552: error: 'quit_hook' undeclared here (not in a function)
d:/Projects/tome2/src/main.c:675: error: expected identifier or '(' before 'if'
d:/Projects/tome2/src/main.c:679: warning: data definition has no type or storage class
d:/Projects/tome2/src/main.c:679: warning: type defaults to 'int' in declaration of 'signals_init'
d:/Projects/tome2/src/main.c:679: error: conflicting types for 'signals_init'
d:/Projects/tome2/src/externs.h:901: note: previous declaration of 'signals_init' was here
d:/Projects/tome2/src/main.c:682: warning: data definition has no type or storage class
d:/Projects/tome2/src/main.c:682: warning: type defaults to 'int' in declaration of 'init_angband'
d:/Projects/tome2/src/main.c:682: error: conflicting types for 'init_angband'
d:/Projects/tome2/src/externs.h:972: note: previous declaration of 'init_angband' was here
d:/Projects/tome2/src/main.c:685: error: expected identifier or '(' before 'if'
d:/Projects/tome2/src/main.c:688: error: expected declaration specifiers or '...' before numeric constant
d:/Projects/tome2/src/main.c:688: warning: data definition has no type or storage class
d:/Projects/tome2/src/main.c:688: warning: type defaults to 'int' in declaration of 'pause_line'
d:/Projects/tome2/src/main.c:688: error: conflicting types for 'pause_line'
d:/Projects/tome2/src/externs.h:1491: note: previous declaration of 'pause_line' was here
d:/Projects/tome2/src/main.c:691: warning: data definition has no type or storage class
d:/Projects/tome2/src/main.c:691: warning: type defaults to 'int' in declaration of 'play_game'
d:/Projects/tome2/src/main.c:691: warning: parameter names (without types) in function declaration
d:/Projects/tome2/src/main.c:691: error: conflicting types for 'play_game'
d:/Projects/tome2/src/externs.h:871: note: previous declaration of 'play_game' was here
d:/Projects/tome2/src/main.c:698: error: expected declaration specifiers or '...' before '(' token
d:/Projects/tome2/src/main.c:698: warning: data definition has no type or storage class
d:/Projects/tome2/src/main.c:698: warning: type defaults to 'int' in declaration of 'quit'
d:/Projects/tome2/src/main.c:698: error: conflicting types for 'quit'
d:/Projects/tome2/src/z-util.h:80: note: previous declaration of 'quit' was here
d:/Projects/tome2/src/main.c:701: error: expected identifier or '(' before 'return'
d:/Projects/tome2/src/main.c:702: error: expected identifier or '(' before '}' token
d:/Projects/tome2/src/main.c:704:2: error: #endif without #if
make[2]: *** [src/CMakeFiles/tome.dir/main.c.obj] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [src/CMakeFiles/tome.dir/all] Error 2
make: *** [all] Error 2

And sorry about breaking the formatting, the stupid Windows terminal doesn't know where to put line breaks.

Edit: I should also note that making CMake notice SDL is harder than it looks. In theory all you should have to do is open an MSYS window, untar the SDL package, and run "make native", but in practice CMake can't seem to figure out where it is.

[AnonymousHero]

The full SHA1 is

c70d7a8c1fdf97aecc1e2a3a182fa1f3c091520a

and it definitely is on master. Maybe you need to pull?

Please try with a fresh clone -- you can clone directly from git@gitorious.org:tome2/tome2.git to your local machine. (I.e. you don't need to make a clone on Gitorious.)

EDIT: Oh, and the error message is for a completely innocuous line in main.c (as of HEAD in master) -- I think you may have a merge conflict there (which git should have told you about).