Re: [pgAdmin][RM4701] Webpack optimization

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