Re: [pgAdmin][RM4701] Webpack optimization

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
Cc: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin][RM4701] Webpack optimization
Date: 2019-10-10 06:39:05
Message-ID: CANxoLDdF1exJ-Aa5qPmjNbr5oNn3_CwmrWDKLQ7sbUE1H2wXOg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Thanks, Patch applied.

On Wed, Oct 9, 2019 at 2:57 PM Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:

> Patch looks good to me.
>
> On Wed, Oct 9, 2019 at 12:02 PM Aditya Toshniwal <
> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>
>> Hi Hackers,
>>
>> Attached is the rebased patch after the latest git pull.
>> Kindly review.
>>
>> On Mon, Oct 7, 2019 at 4:34 PM Aditya Toshniwal <
>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>
>>> 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"
>>>
>>
>>
>> --
>> Thanks and Regards,
>> Aditya Toshniwal
>> Sr. Software Engineer | EnterpriseDB India | Pune
>> "Don't Complain about Heat, Plant a TREE"
>>
>

--
*Thanks & Regards*
*Akshay Joshi*

*Sr. Software Architect*
*EnterpriseDB Software India Private Limited*
*Mobile: +91 976-788-8246*

In response to

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2019-10-10 12:30:09 pgAdmin 4 commit: popen() function strips the quotes from the arguments
Previous Message Akshay Joshi 2019-10-10 06:38:18 pgAdmin 4 commit: Optimize Webpack to improve overall performance.