Re: Fixed RM #1356

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
Cc: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: Fixed RM #1356
Date: 2016-06-16 11:12:57
Message-ID: CA+OCxoxwUuY8E2Tp1Tync=Xg=cB1ktKutGOg5e8ymVccfxV8xA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

On Thu, Jun 16, 2016 at 12:04 PM, Akshay Joshi <
akshay(dot)joshi(at)enterprisedb(dot)com> wrote:

> Hi Dave
>
> On Thu, Jun 16, 2016 at 2:42 PM, Akshay Joshi <akshay(dot)joshi(at)enterprisedb
> .com> wrote:
>
>>
>>
>> On Thu, Jun 16, 2016 at 2:35 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>> Thanks, patch applied.
>>>
>>> However, whilst I was testing, I saw just how slow the tool is:
>>>
>>> SELECT * FROM pg_attribute
>>>
>>> In a PEM database, returns 8150 rows. In pgAdmin 3, this is timed at
>>> 676ms on my laptop. In pgAdmin 4, the busy spinner runs for approx 5
>>> seconds, then the whole UI freezes. I then have to wait a further 3 minutes
>>> and 46 seconds(!!!!) for the operation to complete. Once loaded, scrolling
>>> is very sluggish.
>>>
>>> Please make this your top priority - and if you have incremental
>>> improvements, send them as you have them.
>>>
>>
>> Sure.
>>
>
> Below is my initial finding while running "SELECT * FROM
> pg_attribute" on PEM database, returns 8498 rows:
>
> - Fetching data from the server side took consistent time and it took
> 3-4 secs.
> - Create/Render Backgrid without pagination : *1 minute*
> - Create/Render Backgrid with pagination (50 items per page): *469ms*
> - Create/Render Backgrid with pagination (500 items per page): *3 secs*
> - Create/Render Backgrid with pagination (1000 items per page): *6
> secs*
> - Create/Render Backgrid with pagination (3000 items per page):
> *22 secs*
> - Create/Render Backgrid with pagination (5000 items per page): *36
> secs*
>
>
OK, so I guess diving into Backgrid is the next step. Are there any
profiling tools that could be used?

>
> -
>
>
>>> On Wed, Jun 15, 2016 at 4:50 PM, Akshay Joshi <
>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi
>>>>
>>>> Please ignore the last patch. Attached is the latest patch. Please
>>>> review it.
>>>>
>>>> On Wed, Jun 15, 2016 at 9:10 PM, Akshay Joshi <
>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi
>>>>>
>>>>> On Wed, Jun 15, 2016 at 5:54 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Wed, Jun 15, 2016 at 1:16 PM, Ashesh Vashi <
>>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> On Wed, Jun 15, 2016 at 5:44 PM, Akshay Joshi <
>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Wed, Jun 15, 2016 at 5:25 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hi
>>>>>>>>>
>>>>>>>>> On Wed, Jun 15, 2016 at 11:27 AM, Akshay Joshi <
>>>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>
>>>>>>>>>> Hi
>>>>>>>>>>
>>>>>>>>>> I have fixed RM #1356 "Query tool enhancement". I have added the
>>>>>>>>>> logic to preferences which checks the min/max value before setting it. If
>>>>>>>>>> value given by user is less than min value then set it to min value and if
>>>>>>>>>> value is greater then max value then set it to max value.
>>>>>>>>>>
>>>>>>>>>> For "items_per_page" minimum value is 1. Attached is the patch
>>>>>>>>>> file. Please review it.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> That doesn't do what I asked for though. A value of zero should be
>>>>>>>>> acceptable (I did say <= 0, but = 0 is probably better), and mean 'disable
>>>>>>>>> paging' (i.e. display unlimited rows, and completely hide the paging
>>>>>>>>> controls). From the ticket:
>>>>>>>>>
>>>>>>>>> 1) If "Items per page in grid" <= 0, then never page results. Add
>>>>>>>>> a note to that effect on the Preferences pane, and make zero the default
>>>>>>>>> value.
>>>>>>>>>
>>>>>>>>> The patch does appear to implement:
>>>>>>>>>
>>>>>>>>
>>>>>>>> Backbone's Pageble Collection don't allow Zero value. Below is
>>>>>>>> the check present in "backbone.paginator.js" file:
>>>>>>>>
>>>>>>>> if (pageSize < 1) {
>>>>>>>> throw new RangeError("`pageSize` must be >= 1");
>>>>>>>> }
>>>>>>>>
>>>>>>> We can avoid using the Pageable Collection in our code, when
>>>>>>> pageSize is <= 0 to fix the issue.
>>>>>>>
>>>>>>
>>>>>> Exactly. The entire point of the change is to default to not paging
>>>>>> at all.
>>>>>>
>>>>>> BTW, this may be helpful (also missed from the original patch):
>>>>>>
>>>>>> help_str=gettext('The number of rows to display per page in the
>>>>>> results grid. A value of 0 will disable paging.'),
>>>>>>
>>>>>
>>>>> Fixed above as per suggestion. Attached is the modified patch file.
>>>>> Please review it.
>>>>>
>>>>>>
>>>>>> --
>>>>>> Dave Page
>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>> Twitter: @pgsnake
>>>>>>
>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>> The Enterprise PostgreSQL Company
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> *Akshay Joshi*
>>>>> *Principal Software Engineer *
>>>>>
>>>>>
>>>>>
>>>>> *Phone: +91 20-3058-9517 <%2B91%2020-3058-9517>Mobile: +91
>>>>> 976-788-8246*
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> *Akshay Joshi*
>>>> *Principal Software Engineer *
>>>>
>>>>
>>>>
>>>> *Phone: +91 20-3058-9517 <%2B91%2020-3058-9517>Mobile: +91 976-788-8246*
>>>>
>>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>>
>>
>> --
>> *Akshay Joshi*
>> *Principal Software Engineer *
>>
>>
>>
>> *Phone: +91 20-3058-9517 <%2B91%2020-3058-9517>Mobile: +91 976-788-8246*
>>
>
>
>
> --
> *Akshay Joshi*
> *Principal Software Engineer *
>
>
>
> *Phone: +91 20-3058-9517 <%2B91%2020-3058-9517>Mobile: +91 976-788-8246*
>

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Harshal Dhumal 2016-06-16 11:14:21 Re: Fix for issue RM1336 [pgadmin4]
Previous Message Akshay Joshi 2016-06-16 11:04:56 Re: Fixed RM #1356