Re: [pgAdmin][RM4701] Webpack optimization

From: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>
To: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
Cc: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin][RM4701] Webpack optimization
Date: 2019-09-30 06:09:01
Message-ID: CAM9w-_nN8oAh5aJ+i7cvXXaqkA5+LC0MoHFMb-JEds4AVnRuUA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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.

> 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.
>
>> - *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
>>
>>
>>
>> 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"

Attachment Content-Type Size
RM4701_v3.patch application/octet-stream 208.1 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Khushboo Vashi 2019-09-30 12:36:19 Re: [pgAdmin][RM4701] Webpack optimization
Previous Message Dave Page 2019-09-27 10:16:11 pgAdmin 4 commit: PEP-8 fix.