Re: [pgAdmin][RM4701] Webpack optimization

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"