Re: [pgAdmin][RM4701] Webpack optimization

From: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>
To: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Cc: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
Subject: Re: [pgAdmin][RM4701] Webpack optimization
Date: 2019-10-07 11:04:25
Message-ID: CAM9w-_=ZhsAareA9ww5502fLbYrzbdN38rixokmCmH5tAwjsvw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Hackers,

Attached is the updated patch. Jasmine test cases were failing randomly.
They are fixed now.

Kindly review.

On Thu, Oct 3, 2019 at 12:40 PM Aditya Toshniwal <
aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:

> Hi Hackers,
>
> Attached is the updated patch. Debugger and Jasmine test cases are fixed.
> However, API test cases did not fail for me. Tried on PG 9.4, 11 both
> server mode and desktop mode.
> Kindly review.
>
> On Tue, Oct 1, 2019 at 2:23 PM Aditya Toshniwal <
> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>
>> Hi,
>>
>> Seems like things are breaking :(
>> I had tested jasmine and API at least and were working fine.
>> Will send an updated patch.
>>
>> On Tue, Oct 1, 2019 at 1:25 PM Khushboo Vashi <
>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Aditya,
>>>
>>> - Debugger is not working.
>>> - 14 Jasmine test cases are failing.
>>> - Also, please check the API test cases once as they are failing on my
>>> machine but may be this could be a path issue.
>>>
>>> Thanks,
>>> Khushboo
>>>
>>> On Tue, Oct 1, 2019 at 12:25 PM Aditya Toshniwal <
>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi,
>>>>
>>>> Attached is the updated patch.
>>>> Kindly review.
>>>>
>>>> On Mon, Sep 30, 2019 at 6:06 PM Khushboo Vashi <
>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> On Mon, Sep 30, 2019 at 11:39 AM Aditya Toshniwal <
>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Attached is the updated patch. Also includes few minor changes to
>>>>>> make it work on IE.
>>>>>>
>>>>>>
>>>>>> On Thu, Sep 26, 2019 at 4:16 PM Aditya Toshniwal <
>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi Khushboo,
>>>>>>>
>>>>>>> On Thu, Sep 26, 2019 at 3:57 PM Khushboo Vashi <
>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> Hi Aditya,
>>>>>>>>
>>>>>>>> I have reviewed the patch and it works fine, however some of the
>>>>>>>> review comments as below:
>>>>>>>>
>>>>>>>> - Flask compress dependency is not included in the requirement.txt
>>>>>>>> file in this patch, so giving error.
>>>>>>>>
>>>>>>> I had added that I remember :/
>>>>>>> Bad miss :(
>>>>>>>
>>>>>> Added.
>>>>>>
>>>>>>> - I have found 2 forms of sprintf in the code as below, can we make
>>>>>>>> them identical?
>>>>>>>> sprintf(gettext('Restore (%s: %s)'), node.label, data.label)
>>>>>>>> pgadminUtils.sprintf('%s/%s', ref, encodeURI(o._id))
>>>>>>>>
>>>>>>> I checked this. The first one is imported in an ES6 syntax file,
>>>>>> which allows only sprintf to be imported. The second one is in AMD file,
>>>>>> where the complete file has to be imported. They cannot be identical.
>>>>>>
>>>>> Okay.
>>>>>
>>>>>> Yeah, I will check what I can do on this.
>>>>>>>
>>>>>>>> - I couldn't find the way to test the Flask - Compress scenario,
>>>>>>>> please let me know how to check
>>>>>>>>
>>>>>>> OK, so as per suggestions, Flask Compress is disabled in Desktop
>>>>>>> Mode. In the server mode, you can check the headers in network.
>>>>>>>
>>>>>> The Flask Compress Mode is disabled in Server Mode and working in
>>>>> Desktop Mode.
>>>>> This condition is the culprit : if not config.DEBUG and not
>>>>> config.SERVER_MODE:
>>>>>
>>>>>> - *Webpack changes including lazy loading looks good to me*, however
>>>>>>>> I couldn't find any performance improvement in lighthouse, may be because
>>>>>>>> there are many parameters which are in consideration as per their report.
>>>>>>>>
>>>>>>> The problem with lighthouse is, it only checks the initial loading
>>>>>>> of the app. We have improved lazy loading in SQL editor, which is not even
>>>>>>> tested by lighthouse \_(- -)_/
>>>>>>>
>>>>>>> Thank you, will send the updated patch.
>>>>>>>
>>>>>>> Thanks,
>>>>> Khushboo
>>>>>
>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Khushboo
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Wed, Sep 25, 2019 at 12:58 PM Aditya Toshniwal <
>>>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> I've also added a yarn command in package.json - "yarn run
>>>>>>>>> bundle:analyze" which will generate an html report (
>>>>>>>>> webpack-bundle-analyzer) along with bundling for production.
>>>>>>>>>
>>>>>>>>> On Wed, Sep 25, 2019 at 12:46 PM Aditya Toshniwal <
>>>>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>>>>>
>>>>>>>>>> Hi Hackers,
>>>>>>>>>>
>>>>>>>>>> Attached is the updated patch to incorporate the reviews.
>>>>>>>>>> I'm finally able to lazy load chunks of JS ( leaflet,
>>>>>>>>>> wkx, snapsvg, flotr2 - after a lot of hair pulling) as required to further
>>>>>>>>>> reduce initial load time.
>>>>>>>>>> gzip will be done only in case of server mode as suggested. Below
>>>>>>>>>> is the new structure of generated files (gzip will further reduce the size):
>>>>>>>>>> [image: Screenshot 2019-09-25 at 12.34.44.png]
>>>>>>>>>> Kindly review.
>>>>>>>>>>
>>>>>>>>>> On Wed, Sep 18, 2019 at 5:28 PM Ashesh Vashi <
>>>>>>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>
>>>>>>>>>>> On Wed, Sep 18, 2019 at 5:25 PM Aditya Toshniwal <
>>>>>>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi Ashesh,
>>>>>>>>>>>>
>>>>>>>>>>>> On Wed, Sep 18, 2019 at 5:05 PM Ashesh Vashi <
>>>>>>>>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> On Wed, Sep 18, 2019 at 2:26 PM Aditya Toshniwal <
>>>>>>>>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi Hackers,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Attached is the patch to optimise webpack bundled files for
>>>>>>>>>>>>>> improving performance.
>>>>>>>>>>>>>> @Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com> Thank you for
>>>>>>>>>>>>>> your suggestions and inputs.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Changes include:
>>>>>>>>>>>>>> 1) Remove underscore-string and sprintf-js packages as we
>>>>>>>>>>>>>> were using only %s. Instead, added a function to do the same. Also changed
>>>>>>>>>>>>>> gettext to behave like sprintf directly.
>>>>>>>>>>>>>> 2) backgrid.sizeable.columns was not used anywhere, removed.
>>>>>>>>>>>>>> @babel/polyfill is deprecated, replaced it with core-js.
>>>>>>>>>>>>>> 3) Moved few css to make sure they get minified and bundled.
>>>>>>>>>>>>>> 4) Added Flask-Compress to send static files as compressed
>>>>>>>>>>>>>> gzip. This will reduce network traffic and improve initial load time for
>>>>>>>>>>>>>> pgAdmin.
>>>>>>>>>>>>>> 5) Split few JS files to make code reusable.
>>>>>>>>>>>>>> 6) Lazy load few modules like leaflet, wkx is required only
>>>>>>>>>>>>>> if geometry viewer is opened. snapsvg loaded only when explain plan is
>>>>>>>>>>>>>> executed. This will improve sqleditor initial opening time.
>>>>>>>>>>>>>>
>>>>>>>>>>>>> Move svgsnap, and its dependencies completely in separate
>>>>>>>>>>>>> module, and load them only when required (lazy loading).
>>>>>>>>>>>>> Similarly - leaflet should be in a separate module, as not
>>>>>>>>>>>>> many user will use the Geometry Viewer.
>>>>>>>>>>>>>
>>>>>>>>>>>> They are lazy loaded in the UI component, but are bundled along
>>>>>>>>>>>> with sqleditor JS. I'll separate them into a different JS file to further
>>>>>>>>>>>> isolate.
>>>>>>>>>>>>
>>>>>>>>>>> Code is written to loading it lazily, but - bundles are not made
>>>>>>>>>>> like that.
>>>>>>>>>>> Hence - whenever you load the sqleditor bundle, it will load all
>>>>>>>>>>> dependencies, which is not exactly expected in lazy loading.
>>>>>>>>>>>
>>>>>>>>>>> -- Ashesh
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>> -- Thanks, Ashesh
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Below is the size and files difference after and before the
>>>>>>>>>>>>>> changes:
>>>>>>>>>>>>>> [image: Screenshot 2019-09-18 at 14.13.02.png][image:
>>>>>>>>>>>>>> Screenshot 2019-09-18 at 14.04.41.png]
>>>>>>>>>>>>>> Below is the lighthouse (Chrome extension) report after and
>>>>>>>>>>>>>> before the changes:
>>>>>>>>>>>>>> [image: Screenshot 2019-09-18 at 13.53.10.png] [image:
>>>>>>>>>>>>>> Screenshot 2019-09-18 at 14.08.20.png]
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Do not forget to do a yarn install and a pip install
>>>>>>>>>>>>>> Kindly review.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> --
>>>>>>>>>>>>>> Thanks and Regards,
>>>>>>>>>>>>>> Aditya Toshniwal
>>>>>>>>>>>>>> Software Engineer | EnterpriseDB India | Pune
>>>>>>>>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> --
>>>>>>>>>>>> Thanks and Regards,
>>>>>>>>>>>> Aditya Toshniwal
>>>>>>>>>>>> Software Engineer | EnterpriseDB India | Pune
>>>>>>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> Thanks and Regards,
>>>>>>>>>> Aditya Toshniwal
>>>>>>>>>> Software Engineer | EnterpriseDB India | Pune
>>>>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Thanks and Regards,
>>>>>>>>> Aditya Toshniwal
>>>>>>>>> Software Engineer | EnterpriseDB India | Pune
>>>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Thanks and Regards,
>>>>>>> Aditya Toshniwal
>>>>>>> Sr. Software Engineer | EnterpriseDB India | Pune
>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Thanks and Regards,
>>>>>> Aditya Toshniwal
>>>>>> Sr. Software Engineer | EnterpriseDB India | Pune
>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>
>>>>>
>>>>
>>>> --
>>>> Thanks and Regards,
>>>> Aditya Toshniwal
>>>> Sr. Software Engineer | EnterpriseDB India | Pune
>>>> "Don't Complain about Heat, Plant a TREE"
>>>>
>>>
>>
>> --
>> Thanks and Regards,
>> Aditya Toshniwal
>> Sr. Software Engineer | EnterpriseDB India | Pune
>> "Don't Complain about Heat, Plant a TREE"
>>
>
>
> --
> Thanks and Regards,
> Aditya Toshniwal
> Sr. Software Engineer | EnterpriseDB India | Pune
> "Don't Complain about Heat, Plant a TREE"
>

--
Thanks and Regards,
Aditya Toshniwal
Sr. Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"

Attachment Content-Type Size
RM4701_v6.patch application/octet-stream 477.3 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2019-10-08 08:15:31 Re: PATCH: RM #4778 - Query Plan Analyser
Previous Message Akshay Joshi 2019-10-07 11:03:07 pgAdmin 4 commit: Ensure backup a partition table should not backup the