#45602: is_..._in_range() with a standardized fingerprint
Open Date: 2022-09-07 17:44
Last Update: 2022-10-29 13:28
URL for this Ticket:
https://osdn.net//projects/freeciv/ticket/45602
RSS feed for this Ticket:
https://osdn.net/ticket/ticket_rss.php?group_id=12505&tid=45602
---------------------------------------------------------------------
Last Changes/Comment on this Ticket:
2022-10-29 13:28 Updated by: alienvalkyrie
Comment:
Reply To alienvalkyrie
Reply To cazfi
Have you been able to run any profiling for this? To see that this really has the effect we hope (or at least; that there's no factor that we have missed that would cause this to be less efficient)
I have not. I was going to run some autogames today, both to make sure I didn't break anything, and to get a look at the turn change times.
Looks like nothing broke, but it doesn't seem to be consistently quicker or slower. Though I did just realize I ran it on an unoptimized (debug) build, so I'm not sure that means much.
@cazfi if you want to do some proper profiling, feel free to take over this ticket; I don't currently have the necessary builds and tools set up.
For the record, I do think this change is already worth it for the sake of abstraction and consistency, irrespective of efficiency gains; so it would still be justified to push it even without profiling data to back it up.
---------------------------------------------------------------------
Ticket Status:
Reporter: cazfi
Owner: alienvalkyrie
Type: Patches
Status: Open [Owner assigned]
Priority: 5 - Medium
MileStone: 3.1.0
Component: General
Severity: 5 - Medium
Resolution: Accepted
---------------------------------------------------------------------
Ticket details:
Looking to make is_req_active()
I think the main problem is that those 'case's in the switch are different from each other. The compiled code really need to go "if else if else if else if else if else" to find the block for the requirement type they are interested about. It would be much better to have all cases to call similar callback function -> then we could have those callbacks in an array indexed by the requirement type id.
With the new req_context this makes even more sense than before. is_req_active() can just pass the context pointer as it got it, to the callbacks. There's no "passing parameters needed by the particular callback"
The req_context point makes it clear that the change won't get backported to S3_0, where we don't have that context.
--
Ticket information of Freeciv project
Freeciv Project is hosted on OSDN
Project URL: https://osdn.net/projects/freeciv/
OSDN: https://osdn.net
URL for this Ticket:
https://osdn.net/projects/freeciv/ticket/45602
RSS feed for this Ticket:
https://osdn.net/ticket/ticket_rss.php?group_id=12505&tid=45602