Table: size() function called more times than needed

[Jens Jansson]

The table contains functions that constantly needs the size of the container to perform its duties. You get this size by calling size(). However, size() is called multiple times inside tablefunctions instead of just storing the function and this causes unnecessary function calls. For example refreshRenderedCells() has six occurrences of size() A better solution would be to call size() the first time it is needed inside the function and store it to a temporal variable.

refreshRederedCells() changes would therfore be Line ~1163: int size = size(); and thereafter every occurence of size() -> size

[Jens Jansson]

Or even better solution would be to have one class variable that is update every time something happens to the table

private int size = -1;

setSize(){

this.size = size();

}

and setSize() would be run upon changes to the table

[Matti Tahvonen]

Size changes does not necessary happen through Table component so having internal cache for the size is not a valid solution. refreshRederedCells and paintContent are only places where the value could be cached. If size() is heavy operation in container, the optimization ought to be done there. And even there ought to be compilers and possibly underlaying DB layers (query cache etc) job to do optimization.

Anyway, in [4752] optimized refreshRenderedCells function.

BTW. Was this really a performance issue for you? If not google "premature optimization".

[Marc Englund]

My own thoughts (disclaimer: I did not check Table implementation before writing this):

The original problem+solution is certainly valid: size() should not be called multiple times within one function, since the Container might do expensive stuff to calculate size (the OP actually has a container that checks size from the database, AFAIK)

Solution 2 should also be valid, and theoretically even better: Containers should fire a ItemSetChangeEvent? if size has changed - but this is actually hard to implement (how do you know when the db has changed), so we'll probably introduce a lot of bugs if we do it this way (since it changes the way Table discovers container changes).

Generally we should assume Container.size() is slow and try to call it as seldom as possible.

A compromise would be to cache size as suggested in solution 2, but also call Container.size() once each time we render the table (or at some other appropriate time).

[Matti Tahvonen]

Quick notes in case the issue is some day reopened:

• ItemSetChangeEvent? cannot always be implemented (case: two applications using same DB)
• "Solution 2" is definitely better than introducing local variables for funtion level caching (Case: function caches size, fires event, listener changes container size, function used old size)
• haven't either checked if there still exits places for optimization
• Table's Millstone era server side component ought to be ditched in favor of new implementation. If you really start to look into the code you'll find that it smells.