alpheios-project / alpheios-core

Alpheios Core Javascript Packages and Libraries
16 stars 2 forks source link

FF: script message generated when double click from wordlist #136

Open monzug opened 4 years ago

monzug commented 4 years ago

build 3.0.0.91: I got the yellow browser message (a script is running) in FF. to repro double-click on paenitere or sum in https://it.wikipedia.org/wiki/Verbi_latini_deponenti_e_semideponenti
then go to inflection table then go to wordlist and limit by this session. then double click on the same word. the yellow message will be generated.

balmas commented 4 years ago

I am able to reproduce this with both the 3.0.3.91 build and the current release version 3.0.1.82

So I don't think it's anything new with this incremental, but there's definitely something consuming a lot of memory and/or processing units here.

I can reproduce on the the latin library, if I lookup 'paenitere' in the lookup box and then follow the same steps as above.

Other words don't demonstrate the same extreme behavior, but generally it does seem that verbs are performing worse here than nouns so I would guess it has something to do with the resources required to produce the Latin verb inflection tables.

@irina060981 can you investigate a bit to see if you can narrow this down any further? Thanks!

irina060981 commented 4 years ago

I was able to reproduce the fault of FF only after I have added more words to Wordlists - latin and greek. And as I could see that the problem is not in RAM - we have a great grow of CPU - I had it up to 10%. So we have some heavy calculations somewhere that depends on the amount of words. After some time CPU went to 0% as before.

irina060981 commented 4 years ago

But I was able to reproduce it only once. And I could see that we have a CPU increase on each Lexical Query up to 8-11% but not each time. The biggest amount - when we need to wait for result longer time - it could be progressbar or some behind the scene processes. A light grow on openning inflections.

We have a lot of parallel processes behind the main code workflow. Much of them couldn't be easily trackable - events handling, Vuex storage, modules inject and may be something else. Once I tried before to find the problem (there was a similiar issue) - but if it is still reproducable, then it has the more complex nature.

The previous time - I removed features one by one, found that there was some heavy operations (for RAM) with inflections and Progress Bar. Now it is for CPU. if it is reproducable only if there are more than some amount of words (when I have less then 10 words for Latin list - I didn't have CPU much then 4-5% when the word was retrieved). Then we could have some problems with the current data handling (not garbage).

I have found such an issue of Vuex https://github.com/vuejs/vuex/issues/1338 https://github.com/vuejs/vuex/issues/1139 https://github.com/vuejs/vuex/issues/659

We have the same - we have a lot of state attributes, mutations and commits. In our build we have strict mode.

According to this as we use ES6 than it is added automaticaly https://stackoverflow.com/questions/38664229/disable-babel-strict-mode-from-webpack-config-js

But I am not sure if this is a real problem in our case.

@balmas and @kirlat , what do you think?

balmas commented 4 years ago

Hmm. I wonder if the culprit might be the interaction with the indexeddb? We update it several times with each query. Maybe we should test with the wordlist disabled. We could add a config switch to turn it off for testing purposes.

irina060981 commented 4 years ago

We have the most CPU utilization in the combination - retrieve data from wordlist and by lexicalQuery (from your tests and my tests)

We could turn off the Wordlist property - but we couldn't be sure what has difficult calculations:

They are tightly connected.

kirlat commented 4 years ago

@irina060981, it is a very good catch about a strict mode. We have enabled it on Vuex in a UI controller https://github.com/alpheios-project/components/blob/master/src/lib/controllers/ui-controller.js#L127 (with the comment to disable in production 🙂). Maybe we can disable it and see if it will lead into any performance improvements? That might bring some results quickly.

kirlat commented 4 years ago

I think the strict mode that webpack sets is different from the one used by Vuex. Webpack is probably (if statement in the link still holds true because two years passed and Wepack change a lot since then) forcing all code within ES modules to run in strict mode, as all exported ES modules should run in this mode by default (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/export). The strict mode of Vuex is applied to Vuex only and adds extra overhead by controlling function calls.

monzug commented 4 years ago

@kirlat, is the environment the same as release date or something has changed in between? I have noticed an increased number of script messages recently.

On Tue, Nov 19, 2019 at 4:53 PM kirlat notifications@github.com wrote:

I think the strict mode that webpack sets is different from the one used by Vuex. Webpack is probably (if statement in the link still holds true because two years passed and Wepack change a lot since then) forcing all code within ES modules to run in strict mode, as all exported ES modules should run in this mode by default ( https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/export). The strict mode of Vuex is applied to Vuex only and adds extra overhead by controlling function calls.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/alpheios-project/components/issues/858?email_source=notifications&email_token=AJ32UOOEOQ7VW5GYDXR5VC3QUQDX3A5CNFSM4JOTQ3FKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEOVQDI#issuecomment-555571213, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJ32UOJRNYSZKB6MPXZWJFDQUQDX3ANCNFSM4JOTQ3FA .

kirlat commented 4 years ago

@kirlat, is the environment the same as release date or something has changed in between? I have noticed an increased number of script messages recently.

@monzug I believe it is the same. I can't think of anything that was changed recently.

balmas commented 4 years ago

I agree this hasn't changed recently but will give @monzug a build with it set to off to see if it makes a difference.

kirlat commented 4 years ago

I wonder if the culprit might be the interaction with the indexeddb?

I second to that. From my recent experience with CEDICT IndexedDB can definitely take its toll on performance, especially if a wait to the results of the call blocks the other activity. JS is single threaded (except for WebWorkers) and potentially anything can slow the main loop down causing a waterfall effect down the road.

It would be cool if we could disable IndexedDB temporarily and see if it will give us anything. Could we just disable all permanent (IndexedDB/Remote DB) storage for a moment?

kirlat commented 4 years ago

On my opinion, performance testing is more art than science, unfortunately. We could get results with browser's performance tools, but it require very scrupulous attention and it is hard to get results when multiple pieces of code are involved simultaneously, as @irina060981 noticed.

There is one kind of tools that we haven't employed yet: browser-based testing tools, like Nightmare http://www.nightmarejs.org/. Since nothing is emulated except for user actions it can in theory provide results very close to real life. With browser automation we can run multiple queries on certain words within a limited amount of time and thus it can be an opportunity to discover potential bottlenecks. @balmas, @irina060981, what do you think?

balmas commented 4 years ago

It would be cool if we could disable IndexedDB temporarily and see if it will give us anything. Could we just disable all permanent (IndexedDB/Remote DB) storage for a moment?

The more I think about this though, I am not sure if indexeddb could be the culprit. I was getting the script message even when not logged in, and we only engage with indexeddb once logged in.

balmas commented 4 years ago

There is one kind of tools that we haven't employed yet: browser-based testing tools, like Nightmare http://www.nightmarejs.org/. Since nothing is emulated except for user actions it can in theory provide results very close to real life. With browser automation we can run multiple queries on certain words within a limited amount of time and thus it can be an opportunity to discover potential bottlenecks. @balmas, @irina060981, what do you think?

I think it could be useful. I'm not sure of the priority of this right now.

balmas commented 4 years ago

I have created build qa-3.0.3.92 which has the strict mode flag turned off, but I was still able to reproduce the ff script error with just one lookup on paenitere, while logged out.

balmas commented 4 years ago

It looks to me like it's the ViewSetFactory.create call that takes the longest here. Interesting to note that we get the script error more often with the lookup from the wordlist rather than the direct lookup.

One difference here is that when we do a direct lookup, the UI gets updated immediately with the lexical query loading progress bar, but if we do the lookup from the wordlist, when it's a word that was added in the current session, if the popup is already open we don't display that message,

I think that's because we skip over the actual execution of the query since the full homonym object is loaded from memory. So there is no update the UI before initiating the VewSetFactory.create call. This might be why we get the FF warning message in that case, because there is no feedback to the user while we are waiting for that to return, whereas when the query itself is executed, we are immediately responding with the opening of the popup with the loading message.

@monzug I would be curious to know if you get the ff warning message in both situations -- when the popup is already open when you double-click on the word from the wordlist, and when it isn't.

monzug commented 4 years ago

build 3.0.3.91, I was able to generate the script error in the following situation: 1) double click on paenitere verb, 2) from wordlist with limit to session with pop-up already open and 3) from wordlist with limit to session with pop-up not open. build qa-3.0.3.92, no script message generated on double click on the verb, but I got the script message when double click from wordlist in both situation (tested not logged in and logged in) will test also on the release version

monzug commented 4 years ago

in Alpheios Reading Tools 3.0.1.82, no script message generated on double click on the verb, but I got the script message when double click from wordlist in both situation with or without pop-up (tested not logged in and logged in)

balmas commented 4 years ago

ok, it needs more investigation but I don't think it should hold up the incremental, do you agree?

monzug commented 4 years ago

agree. it's also in the current release version

On Wed, Nov 20, 2019 at 10:47 AM Bridget Almas notifications@github.com wrote:

ok, it needs more investigation but I don't think it should hold up the incremental, do you agree?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/alpheios-project/components/issues/858?email_source=notifications&email_token=AJ32UOLIHZISAIPDGYLOZM3QUVLYVA5CNFSM4JOTQ3FKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEESNXYA#issuecomment-556063712, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJ32UOJWJMQEBBRDDSVQOXTQUVLYVANCNFSM4JOTQ3FA .

irina060981 commented 3 years ago

@balmas and @monzug , may be it is worth to retest it after all made refactoring? (I am checking through my old issues :) )

balmas commented 3 years ago

Yes I agree. Assigning to @monzug for some testing on the latest dev build.

monzug commented 3 years ago

I got the yellow message when double clicking on paenitere from wordlist. FF (84.0.1 and 85): Alpheios Reading Tools 3.3.3 build 20210122645 Alpheios Components 3.3.3-20210122585