Re: Query grid

Dave Page wrote:
>
>
>
>>-----Original Message-----
>>From: Andreas Pflug [mailto:pgadmin(at)pse-consulting(dot)de]
>>Sent: 03 April 2006 14:22
>>To: edigeronimo(at)xtracards(dot)com; Dave Page
>>Cc: pgadmin-hackers
>>Subject: Query grid
>>
>>After several weeks I succeeded in reconnecting my win32
>>machine and have a look at pgadmin stuff again. I fired up
>>the query tool and my first impression was "can we have this
>>a little less ugly please?" when I saw those grey spaces. Is
>>the tool expected to display up to 10G of rows?
>
>
> Well, those ugly grey spaces look identical to those in your View Data
> code so any fix should be applied to both controls.
>
>
>>I remember the comment how fast this should have become, so I
>>tried "SELECT * from pg_attribute" on a small db. I had 3614 rows in
>>600+1400ms on a pre-grid version (ancient 700MHz Athlon), and 720 rows
>>in 600+47000ms on the grid version, with 2894 rows
>>dropped!!!! I killed another attempt after 5 minutes.
>
>
> Yes, it was much faster. See the benchmarks I posted to the list at
> http://www.pgadmin.org/archives/pgadmin-hackers/2006-02/msg00155.php
>
> [For a 100,000 row query]
> 52.277 Secs + 9.123 Secs (v1.5, with your patch)
> 52.276 Secs + 39.688 Secs (v1.4.1)
>
> However, I am seeing dismal performance today, so something has got
> borked at some point.
>
>
>>IIRC, Edward mentioned he used the original wxGridTable
>>implementation because it worked good for him, and
>>consequently I didn't find any
>>SetTable() call. As I mentioned earlier and , an inevitable
>>requirement is the usage of virtual row/col management, to
>>improve performance on larger resultsets. The new ctlSqlGrid
>>fails to implement it, and thus fails even on result sets
>>that can't really be called big.
>
>
> Originally it was a virtual table, however it was found to be faster yet
> to use the built in table. I wonder now however, if when I tested the
> later version of the patch I was actually testing the wrong version.
> Maybe that needs to be reimplemented.

The builtin table can *never* be faster than a clean virtual implementation.

>
>
>>This implementation is clearly a bad regression. Please
>>revert ASAP and come back with a non-grid version (virtual
>>listbox or listview), I still fail to see any advantage of
>>wxGrid for this usage.
>
>
> It has been proven to be significantly faster. I will not revert the
> patch unless it proves impossible to fix the problem you are seeing,
> which seems unlikely given the speedup which I saw on the earlier
> versions. Reverting every patch because it is found not to be perfect
> when first applied would be a very good way of ensuring that nothing
> ever gets done.

IMNSHO this patch is rotten from the ground. wxGrid is known to be
flakey, extending its use is a bad idea. The speed issue is not a
question of grid or non-grid, it's a question of virtual data/display
management.

Regards,
Andreas