Re: [pgAdmin][RM4701] Webpack optimization

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