patchFreeciv - Patches: patch #4949, [Metaticket] Get rid of massive...

 
 
Show feedback again

patch #4949: [Metaticket] Get rid of massive switch-cases

Submitted by:  Marko Lindqvist <cazfi>
Submitted on:  Mon 14 Jul 2014 08:17:00 AM UTC  
 
Category: NonePriority: 5 - Normal
Status: NonePrivacy: Public
Assigned to: NoneOpen/Closed: Open
Planned Release: 

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Mon 17 Nov 2014 11:26:16 PM UTC, comment #2:

If your var is one of the "specenum" thingies used here, and you optimize for size, the machine code should be a jump table to the function calls.

20 years old observation, the point being, a compiled switch does not necessarily compare cases sequentially. Code intended for debugging could do this, and Emmet's suggestion would be independent of compiler options. (This old report was linked in bug #22944 about freeciv_web optimizations after 2.5.)

Frank <dunnoob>
Mon 14 Jul 2014 12:49:21 PM UTC, comment #1:

Modern optimising compilers sometimes compile into a branch table, which is precisely as efficient as an array of pointers to pointers to functions (so nearly, but not quite as efficient as an array of pointers to functions), but this isn't always the case, especially in situations where multiple values hit the same code block, or where fallthrough is required (instead choosing a binary search or possibly a sequence of conditionals, depending on the code structure). For the really huge tables, where every possible value has an associated (different) code block, the compiler probably does the right thing, but the code would probably be easier to read and maintain if the functions were smaller and the callers had a constant signature (e.g. in requirements.c, there are all sorts of inconsistencies in whether various conditions are checked in the switch statement or in the function called by the switch statement, which can be confusing when hunting code paths).

My personal preference is that any switch statements that are converted to function arrays maintain a static function array in a .c file which is initialised on program load, rather than hardcoding the arrays in header files and exporting them everywhere. If access is required from other modules, macro accessors can drive a generic handling function that calls into the function array (potentially with localised pre- and/or post-processing).

Emmet Hikory <persia>
Project Member
Mon 14 Jul 2014 08:17:00 AM UTC, original submission:

To at least open a ticket of something I've long wanted to do...

We have a couple of switch - case constructs with a lot of cases (packet handling has a case for each packet number, ai effect evaluation for each effect type). While I haven't measured how much that affects our overall performance, checking against each case in often called functions certainly gives us some performance hit (I don't think any compiler can make major optimizations on this - feel free to correct me).

My plan is to investigate possibilities to build function arrays, so that what currently is an case would be array index:

switch (var) {
case 0:
func0();
break;
case 1:
func1();
break;
case 2:
func2();
break
...
}

->

funcs[var]();

Marko Lindqvist <cazfi>
Project Administrator

 

(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Digest:
   patch dependencies.

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by dunnoob (Posted a comment)
  • -unavailable- added by persia (Posted a comment)
  • -unavailable- added by cazfi (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follows 1 latest change.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 25 Nov 2014 01:13:13 PM UTCpepetoDependencies-=>Depends on patch #5461
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup