Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Quesiton] About the performance of utils.removeItems #3106

Closed
finscn opened this issue Oct 14, 2016 · 5 comments
Closed

[Quesiton] About the performance of utils.removeItems #3106

finscn opened this issue Oct 14, 2016 · 5 comments
Labels
🤔 Question User question, similar to Help Wanted or Needs Help. These can be addressed whenever someone has tim

Comments

@finscn
Copy link
Contributor

finscn commented Oct 14, 2016

I found utils.removeItems is this :

    removeItems: function (arr, startIdx, removeCount)
    {
        var length = arr.length;

        if (startIdx >= length || removeCount === 0)
        {
            return;
        }

        removeCount = (startIdx+removeCount > length ? length-startIdx : removeCount);
        for (var i = startIdx, len = length-removeCount; i < len; ++i)
        {
            arr[i] = arr[i + removeCount];
        }

        arr.length = len;
    },

Why don't use :

arr.splice(startIdx, removeCount);

I think arr.splice is more fast than current utils.removeItems

@finscn
Copy link
Contributor Author

finscn commented Oct 14, 2016

I've tested , use :

    removeItems: function (arr, startIdx, removeCount)
    {
         arr.splice(startIdx, removeCount);
    },

Everything is OK and with better performance .

@englercj englercj added the 🤔 Question User question, similar to Help Wanted or Needs Help. These can be addressed whenever someone has tim label Oct 14, 2016
@englercj
Copy link
Member

Here is some reading:

#2205
#2209
https://gamealchemist.wordpress.com/2013/05/01/lets-get-those-javascript-arrays-to-work-fast/

splice is almost always slower than our removeItems function, but also splice generates a new array every time it runs which can create unnecessary GC burden.

@finscn
Copy link
Contributor Author

finscn commented Oct 14, 2016

Oh, I think maybe different js engine will present different result .
I will do more test again. Thanks .

By the way , the Article at 2013 is too old , maybe the conclusion is out.

@englercj
Copy link
Member

Landscape changes for sure, but like I mentioned the big one for us is that splice creates a new array and we certainly dont want that garbage.

@lock
Copy link

lock bot commented Feb 25, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Feb 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🤔 Question User question, similar to Help Wanted or Needs Help. These can be addressed whenever someone has tim
Projects
None yet
Development

No branches or pull requests

2 participants