Bug #44

Remove SMP code

Added by Radegast over 7 years ago. Updated over 6 years ago.

Status:Fixed% Done:

100%

Priority:ImmediateSpent time:0.20 hour
Assignee:Radegast
Category:Client
Target version:2.71rc1
OS: Arch:

Description

SMP support is present but broken in the vanilla Enemy Territory.

Although it is disputable if there will be any benefits to it (see below), we should backport SMP support from ioquake.

Zachary from the ioquake project:

Regular (id software) Quake 3 supported this, and it worked "ok" on old SMP Pentium 500 Machines back in the day.
So all the SMP code in Quake 3 when the source code was released was designed to improve the game on ancient hardware.
On modern hardware the built-in SMP support on the id software Quake 3 gives little benefit to, none, or degrades, performance.

rsmp.png (143 KB) Dragonji, 24.11.2012 14:04

dualprocessoracceleration.jpg (73.3 KB) TheDushan, 15.01.2013 00:22

multithreadinginet.jpg (89.8 KB) TheDushan, 15.01.2013 00:22

smp.jpg (74.1 KB) TheDushan, 15.01.2013 00:22

experimentswithOpenMP-2.jpg (87.1 KB) TheDushan, 15.01.2013 00:22

experimentswithOpenMP-3.jpg (87.6 KB) TheDushan, 15.01.2013 00:22

61
92
93
94
95
96

Associated revisions

Revision 0b6e3874
Added by Radegast over 7 years ago

client: added experimental SMP support, fixes #44
(to enable this feature do premake4 –enable-smp)

Revision 0f1edbd8
Added by IR4T4 about 7 years ago

excluding some code when –enable-smp is not set by #ifdef SMP refs #44

Revision 475a93cd
Added by IR4T4 about 7 years ago

non SMP_SUPPORT build fix refs #44

Revision fcea630d
Added by Radegast almost 7 years ago

client: let users experiment with FEATURE_SMP, refs #44

Revision dde53aa4
Added by IR4T4 over 6 years ago

renderer: more FEATURE_SMP macros for related code refs #44

Revision f3b840b1
Added by Radegast over 6 years ago

client: removed SMP, fixes #44

Revision 47b6b093
Added by RaFaL over 6 years ago

ui: removed smp code refs #44

Revision 50e9fbd6
Added by IR4T4 over 6 years ago

qcommon: SMP code removal refs #44

History

#1 Updated by Radegast over 7 years ago

  • Category changed from General to Client
  • Status changed from New to Fixed
  • Assignee set to Radegast
  • % Done changed from 0 to 100

This feature needs to be enabled at compile time by running premake with --enable-smp option and then setting r_smp cvar to 1.

#2 Updated by Dragonji about 7 years ago

SMP was present in Q3 engine to increase performance for multi-processors machines. Those still exist but are rarely seen.

I would like to ask what about multi-cores support? It will be a lot of work, I am aware of that, but such a feature will provide much better FPS for most ET players in my opinion.

#3 Updated by Radegast about 7 years ago

TheDushan implemented multicore support in OpenWolf and I vaguely remember he gave me a patch for ET:L (and now his repository is public). I didn’t do anything about it because 1) I don’t understand this stuff and 2) This is a huge thing that would likely results in a few bugs which we don’t need right now. However, we can create a ticket for version version:2.80

#4 Updated by IR4T4 about 7 years ago

  • Subject changed from Fix SMP support to Fix SMP support and exclude smp code by precompiler macro
  • Status changed from Fixed to New

Dragon, as far as I know the OS (running a multi core) already deals with this - see processor affinity.

Radegast, I did change the title of ticket and did open it again:


Set more precompiler marcros for smp code - see #ifdef SMP

Related vars and functions
see
- smpActive
- r_smp
- R_SyncRenderThread()
- R_InitCommandBuffers(void)
- R_ToggleSmpFrame(void)

#5 Updated by Dragonji about 7 years ago

IR4T4 wrote:

Dragon, as far as I know the OS (running a multi core) already deals with this - see processor affinity.

Hmm, but what about the "tweak" which results in more FPS when you set your ET process to use one core only?

#6 Updated by IR4T4 about 7 years ago

Don’t know if this is a tweak ... some installs may have better FPS results if one core is set. However you are already able to set affinity by OS so why should we add such a feature again?

#7 Updated by Dragonji about 7 years ago

IR4T4 wrote:

Don’t know if this is a tweak ... some installs may have better FPS results if one core is set. However you are already able to set affinity by OS so why should we add such a feature again?

I think you don’t understand what I mean.

There is such a cvar in Enemy Territory: Quake Wars:

[Dual/Multi-Core Tweaks]

r_useThreadedRenderer [0,1,2 ] - Added as of the 1.2 Patch, this option allows you to enable multithreading if you have a dual or multi-core CPU, and this can improve the performance of ET:QW. By default it’s disabled (set to 0), but you can set it to either 1 or 2, with a value of 1 locking the renderer to your in-game frames, while 2 allows it to run unlocked. The developers recommend a value of 2 for this variable if you wish to enable it. Note that you must enable it either by inserting it in your autoexec.cfg file, or by entering it in the console prior to the start of a game; it can’t be changed during a game.

I mean that something like this could be introduced into ET:L.

#8 Updated by Radegast about 7 years ago

  • Assignee deleted (Radegast)
  • Priority changed from Normal to High
  • % Done changed from 100 to 80

The game now crashes when compiled without SMP_SUPPORT in RB_ExecuteRenderCommands function → if (!r_smp->integer || data == backEndData[0]->commands.cmds)

#9 Updated by IR4T4 about 7 years ago

Radegast wrote:

The game now crashes when compiled without SMP_SUPPORT in RB_ExecuteRenderCommands function → if (!r_smp->integer || data == backEndData[0]->commands.cmds)

shouldn’t occure anymore

#10 Updated by IR4T4 about 7 years ago

Dragon wrote:

I mean that something like this could be introduced into ET:L.

I do exactly understand what you are talking about but QW is written in C++ and ETL in plain C ... time will tell.

#11 Updated by IR4T4 about 7 years ago

  • Status changed from New to Fixed

#12 Updated by IR4T4 about 7 years ago

  • % Done changed from 80 to 100

#13 Updated by Dragonji almost 7 years ago

This is what I get when starting the game with +set r_smp 1. I dunno what does it mean, however why not to share it with you. Maybe it’s just a false info.

Oh BTW, my CPU is Intel Core 2 Duo.

#14 Updated by IR4T4 almost 7 years ago

r_smp is apple only /* better safe than sorry for now. */

#15 Updated by Dragonji almost 7 years ago

Oh lol.

#16 Updated by Radegast almost 7 years ago

  • Status changed from Fixed to Feedback
  • Target version changed from 2.70rc1 to 2.90
  • % Done changed from 100 to 10

SMP doesn’t work for me on my Mac. However, it is part of the code and for those people who want to test it on other platforms and report their findings I removed the hardcoded disablement.

It will likely cause trouble for most users who use it, but since they have to 1) enable FEATURE_SMP and 2) set r_smp to 1 and they receive several warnings in the process, I think it is quite noob-proof.

Dushan implemented real multithreading in OpenWolf by using OpenMP. Maybe we could try to port it..

#17 Updated by Radegast almost 7 years ago

  • Priority changed from High to Low

#18 Updated by TheDushan over 6 years ago

I had few years ago working R_SMP inside OpenWolf, just how I remember I didn’t get much improvements. It just OpenGL renderer into separate thread. As you can see on first three pictures.
Don’t think that today it will be success to have or to use that. I remember that I had enormous problems with getting it to work on XreaL renderer, while with vanilla it was working perfectly.

Last two pictures are some experimental build of OpenWolf with most of loops moved into parallelism and compiled with OpenMP.
At that time I believed that it I move everything into work-sharing constructs that I could get better and more stable engine.

#19 Updated by Radegast over 6 years ago

  • Subject changed from Fix SMP support and exclude smp code by precompiler macro to Remove SMP code
  • Status changed from Feedback to In Progress
  • Assignee set to Radegast
  • Target version changed from 2.90 to 2.71rc1
  • % Done changed from 10 to 0

After what Dushan said and seeing that Tim Angus @ ioquake also removed SMP support, I don’t think we have any reason to fix/keep it.

#20 Updated by Radegast over 6 years ago

  • Status changed from In Progress to Fixed
  • % Done changed from 0 to 100

#21 Updated by IR4T4 over 6 years ago

  • Status changed from Fixed to In Progress
  • Priority changed from Low to Immediate

R_Init fails here after http://www.etlegacy.com/projects/etlegacy/repository/revisions/f3b840b19e64f6a3ae030a34d0146916b15bb36f

----- R_Init -----
SDL using driver "x11" 
Initializing OpenGL display
Estimated display aspect: 1.600
...setting mode 13:
 1440 900
Available modes: '720x450 840x525 960x600 1440x900 1680x1050 680x384 1360x768 960x540 320x240 400x300 512x384 640x480 700x525 800x600 832x624 1024x768 1152x864 1280x960 1400x1050 1280x1024'
Couldn't get a visual
...WARNING: could not set the given mode (13)
Initializing OpenGL display
...setting mode 13:
 1440 900
Available modes: '720x450 840x525 960x600 1440x900 1680x1050 680x384 1360x768 960x540 320x240 400x300 512x384 640x480 700x525 800x600 832x624 1024x768 1152x864 1280x960 1400x1050 1280x1024'
Couldn't get a visual
...WARNING: could not set the given mode (13)
Setting r_mode 13 failed, falling back on r_mode 3
Initializing OpenGL display
...setting mode 3:
 640 480
Available modes: '720x450 840x525 960x600 1440x900 1680x1050 680x384 1360x768 960x540 320x240 400x300 512x384 640x480 700x525 800x600 832x624 1024x768 1152x864 1280x960 1400x1050 1280x1024'
Couldn't get a visual
...WARNING: could not set the given mode (3)
----- CL_Shutdown -----
RE_Shutdown( 1 )
-----------------------
GLimp_Init() - could not load OpenGL subsystem

#22 Updated by Radegast over 6 years ago

Several people tested this on different linux distributions, Windows and Mac and reported that they do not experience any issues. I went through the commit diff once again, but I don’t know what could be wrong with it.

#23 Updated by IR4T4 over 6 years ago

  • Status changed from In Progress to Fixed

OK .. I’ve deleted fs_homepath content and it does work again. I don’t know which cvar caused that ... I assume somemthing went terribly wrong as I did test the code for #175.

Also available in: Atom PDF