RobinHerbots / Inputmask

Input Mask plugin
https://robinherbots.github.io/Inputmask/
MIT License
6.4k stars 2.17k forks source link

Poor performance because of $.extend #1489

Open aetna-softwares opened 7 years ago

aetna-softwares commented 7 years ago

Hi there !

I use inputmask in some heavy pages with a lot of fields and experiences very poor performances on init.

I investigate and the main responsible is the extensive use of $.extend at many places in the code.

I tweak a bit and achieve to get 4x time faster result by getting rid of it at several places. Unfortunately the code seems to be design with the need of clone object so my hack quickly leads to many regressions and fixing them lead again to performance slow down.

It is not strictly speaking a bug and I have no obvious solution to fix it as I think it requires a huge amount of rewriting to get rid of $.extend without breaking everything.

I report it mainly as a reflexion in the case some refactor is planned in the lib, it would be good to keep in mind that cloning object is expensive and should be avoid if possible (and the this.defaults variable is not a small object to clone !...).

for information, here is a quick test showing the bad performance

<input type="button" value="go !" onClick="massInput()"/>
        <div id="container"></div>
function massInput(){
                document.getElementById("container").innerHTML = "" ;
                console.time("create inputs")
                for(var i=0; i<1000;i++){
                    var input = document.createElement("input") ;
                    input.setAttribute("type", "text") ;
                    var optionsDecimal = { radixPoint: "," , groupSeparator : " ", prefix : "", suffix : ""} ;
                    $(input).inputmask('currency', optionsDecimal);

                    document.getElementById("container").appendChild(input) ;

                }
                console.timeEnd("create inputs")
            }

and for information, I workaround the problem by doing a lazy init when user focus in the field with something like this :

    function initField(id){
                var el = document.getElementById(id) ;
                var init = false ;
                el.addEventListener("focus", function(){
                    if(!init){
                        var optionsDecimal = { radixPoint: "," , groupSeparator : " ", prefix : "", suffix : ""} ;
                        $(el).inputmask('currency', optionsDecimal);
                        init = true;
                    }
                }) ;
            }

PS : thanks for the lib, it is the best masking lib by far ;)

RobinHerbots commented 7 years ago

Hi @aetna-softwares ,

Thx for reporting and bring this back to attention. You are using the latest version? Which dependencylib are you using?

There are also extends with deep copy, which I should definitely check if these are necessary. I will run the tests myself and try some optimization.

aetna-softwares commented 7 years ago

I did my test on version 3.3.4 (get via bower, it get jquery 3.1.1 as dependency)

From my hack tests, the deep copy is indeed a part of the problem. My first hack was :

the time was reduced from 4s to 1s

but afterward, I dig further in all extends and start to improve my copy function to check for plain object copy and deep recursion, etc.. and time back to 2s (and regressions bugs appears...)

my observations are :

the possible ways to improve would be to :

But I fear that the second point is too complex to do without breaking compatibility :(