Opened 11 years ago

Closed 11 years ago

#883 closed bug (fixed (in master))

Angband spends too much time updating the item list.

Reported by: morth Owned by: magnate
Milestone: 3.1.1 beta Keywords: cleanup


Well, I accidently overwrote my savefile with my testing svn version, so playing [335056c] (SVN r1444) now... I noticed running is _much_ slower than it was in 3.1.0 beta, even after I disable the pricing log. Almost not playable on my 2GHz macbook pro. Profiling gives the following traceback:

Depth Image Samples Function

24 angband 1034 slay_power
23 angband 1034 object_power
22 angband 1034 object_value_real
21 angband 1034 object_value
20 angband 1034 display_itemlist
19 angband 1034 update_itemlist_subwindow
18 angband 1034 game_event_dispatch
17 angband 1034 redraw_stuff
16 angband 644 process_player
15 angband 644 dungeon
14 angband 644 play_game
13 angband 644 AngbandGame?

It seems a bit obsessive, the item list doesn't need updating at all while I'm not using items from the floor or a monster picks them up/destroys them (this was from running/resting only)

Attachments (1)

itemlist-value.patch (463 bytes) - added by d_m 11 years ago.
Patch which uses k_ptr->cost instead of object_value()

Download all attachments as: .zip

Change History (13)

comment:1 Changed 11 years ago by d_m

This is strange, because AFAIK the itemlist code hasn't been updated between 3.1.0 and HEAD. I know that as of its inclusion the itemlist code was redrawing too frequently, but that's always been the case. Thus, I am pretty sure it's not directly responsible for the slowdown you saw.

That said, it is worth cleaning up.

Anyway, I will try to see what (if anything) has changed in the itemlist drawing code and report back.

comment:2 Changed 11 years ago by anonymous

If you wouldn't mind backing up your savefile, then running 3.1.0-beta again and returning some profiling data from that, it would be helpful.

I compared SVN-1098 (the point where 3.1.0 was made) against HEAD. I didn't notice any changes to object/obj-util.c that affect the itemlist. Of course, it's possible some other code did increase the redrawing frequency. Anyway, it would help to be sure that it is happening more under HEAD than 1098.

comment:3 Changed 11 years ago by d_m

That last comment was by me but I forgot to sign it. Sorry.

comment:4 Changed 11 years ago by morth

Maybe it's drawn just as often, but takes much longer time to do it now. Isn't the call to slay_power new? Since every sample is in that function (sorry for the messed up display), it should be that function that is slow...
That said, it could probably be fixed either by optimizing the function or by calling it less.

Looking a bit at the code, the cache used in slay_power isn't very effective since it's a linear search. It should be sorted or a hash. I haven't verified that that is the actual problem though.

comment:5 Changed 11 years ago by morth

I checked with 3.1.0-beta and display_itemlist is still at the top of the list, but it's way faster.

comment:6 Changed 11 years ago by Taha

This sounds like the same issue documented here:

It definitely was not a problem in 3.10, and has been in all the nightlies I have tried since. I haven't done any detailed followup, except to confirm that the same savefile has the issue in the nightlies but not in 3.10. I have just had to keep the item list turned off since then.

comment:7 Changed 11 years ago by sorear

There are several aspects to this.

object_value does too much; most of the complicated calculation is not needed if the result is just being compared to 0. It should be split out into object_is_worthless and object_value.

The item list is redisplayed every turn.

The item list is sorted using bubble sort, with several calls to object_value in the comparison.

Fixing any one of these would very likely make the game playable again. All three would probably be best (come on, bubble sort??).

comment:8 Changed 11 years ago by magnate

[705b0b1] (SVN r1477) removed the logging of power calculations when object_value is called from sorting the item list (power is now only logged whilst in stores, and during randart generation). Is someone who uses the item list window deems that the game is now playable again, please close this ticket.

comment:9 Changed 11 years ago by magnate

  • Owner set to magnate
  • Status changed from new to assigned

The slowdown is definitely due to the additional calls to the object power function via object_value. My plan is to avoid calling object_value when ordering lists of items, and using k_ptr->cost or e_ptr->cost instead. This will leave the ordering essentially unchanged, and has the added bonus of preventing wands and staves swapping places as you use charges.

comment:10 Changed 11 years ago by magnate

  • Keywords cleanup added

Changed 11 years ago by d_m

Patch which uses k_ptr->cost instead of object_value()

comment:11 Changed 11 years ago by d_m

I'm going to go ahead and email the Angband list to try to get this patch in sooner rather than later.

comment:12 Changed 11 years ago by magnate

  • Resolution set to fixed
  • Status changed from assigned to closed

[7e68087] (SVN r1500)

Note: See TracTickets for help on using tickets.