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

serious performance issue in removeFromParent #14593

Closed
weibobo opened this issue Dec 5, 2015 · 16 comments
Closed

serious performance issue in removeFromParent #14593

weibobo opened this issue Dec 5, 2015 · 16 comments

Comments

@weibobo
Copy link

weibobo commented Dec 5, 2015

environment: windows, lua
repro steps:
1 create a normal layer, for example, a layer with 200 imageview with the same png

     for i = 1, 200 do
        local nn =  ccui.ImageView:create("ui/jiesuan/jiaoyin.png")
        self:addChild(nn)
    end

2 create a button simple to do the create, remove and add action of this layer

        local oldlayer
        function UIHelper.onBtnClick(clean)   
               local cls = require("ui.map.MapListLayer")  
               perfStart()    
               local layer = cls:create()
               perfLog("createMap")   
               if (oldlayer) then       
                      oldlayer:removeFromParent()
                      perfLog("remove old")
                      oldlayer = nil
               end
               gd.UIMgr.UILayer:addChild(layer)
               oldlayer = layer
               return layer
        end

3 you will find the removeFromParent need about 0.2s. The time raises according the node count in layer. Notice that the new layer has not been added to ui yet.
4 Interesting thing is that, if you move the remove action to before cls:create(), the time for removefromparent drop to 0.01s.

@weibobo
Copy link
Author

weibobo commented Dec 5, 2015

by the way, the version is cocos2dx 3.8.1

@weibobo weibobo changed the title serious performance issue in removeParent serious performance issue in removeFromParent Dec 5, 2015
@weibobo
Copy link
Author

weibobo commented Dec 7, 2015

Can someone take a look? or give me some advise about that? It's a common scene that we create a layer, add to ui, then remove the old layer. If the layer is complicated, it might cause up to 0.6s on windows.

@1078559858
Copy link

@xiaofeng11

@xiaofeng11
Copy link
Contributor

@weibobo I'm not familiar with Lua, could you make a simple test for us to test this issue?

For the reason why remove layer with a lot sprite before and after a new layer create has big performance difference, I think this maybe cause by auto release pool, If create layer first, the sprite need to be removed is at middle of auto release pool, remove these item may cause vector use more time to reorder other items.

@weibobo
Copy link
Author

weibobo commented Dec 9, 2015

@xiaofeng11, I don't think it's the auto release pool issue. The layer is removed in the next frame. I'll try to make a c++ test case, but i'm afraid the issue might be connected with lua binding.

@xiaofeng11
Copy link
Contributor

@weibobo Lua is binding from C++ code, you can make a lua test, we'll try to debug it in C++ level to check what's wrong. Thanks.

@weibobo
Copy link
Author

weibobo commented Dec 9, 2015

@xiaofeng11 , hi, i create a simple repo.

  1. create a lua project use version after 3.8.1, cocos new -l lua -d xx
  2. becase we need socket.getime to do performance test, please replace one line in src/config.lua
CC_DISABLE_GLOBAL = false
  1. replace src/app/views/MainScene.lua with the following code:
local MainScene = class("MainScene", cc.load("mvc").ViewBase)

function MainScene:onCreate()
    -- add background image
    display.newSprite("HelloWorld.png")
        :move(display.center)
        :addTo(self)

    -- add HelloWorld label
    cc.Label:createWithSystemFont("Hello World", "Arial", 40)
        :move(display.cx, display.cy + 200)
        :addTo(self)

    local btn = ccui.Button:create()
    btn:setTitleText("remove before 点击我")    
    --btn:setSize(cc.size(100, 100))
    btn:addTouchEventListener(
        function(sender, evtype)
            if (evtype == ccui.TouchEventType.ended) then
                self:onBtnClick()
            end
        end
        ) 
    btn:setPosition(cc.p(300, 200))
    btn:setTitleFontSize(40)
    self:addChild(btn)

    local btn = ccui.Button:create()
    btn:setTitleText("remove after 点击我")

    btn:addTouchEventListener(
        function(sender, evtype)
            if (evtype == ccui.TouchEventType.ended) then
                self:onBtnClick2()
            end
        end
        ) 
    btn:setPosition(cc.p(300, 100))
    btn:setTitleFontSize(40)
    self:addChild(btn)

    local labletm = cc.Label:createWithSystemFont("hello world", "Arial", 40)
    self.labletm = labletm
    self:addChild(labletm)
    labletm:setPosition(cc.p(300, 400))    
    labletm:setString("test")
end

require "socket.core"

local oldlayer
function MainScene:onBtnClick()      
    local tmstart = socket.gettime()
    if (oldlayer) then
        oldlayer:removeFromParent()
        oldlayer = nil
    end
    local newlayer = self:createLayer()        
    self:addChild(newlayer)
    oldlayer = newlayer
    local tmend = socket.gettime()
    local delta = tmend-tmstart
    self.labletm:setString(string.format("use time: %.3f", delta))
end

function MainScene:onBtnClick2()
    local tmstart = socket.gettime()    

    local newlayer = self:createLayer()    

    if (oldlayer) then
        oldlayer:removeFromParent()
        oldlayer = nil
    end
    self:addChild(newlayer)
    oldlayer = newlayer
    local tmend = socket.gettime()
    local delta = tmend-tmstart
    self.labletm:setString(string.format("use time: %.3f", delta))
end

local c = 1
function MainScene:createLayer()   
    c = c + 1 
    local node = cc.Node:create()
    for i=1,300 do
        local s = ccui.ImageView:create("HelloWorld.png")
        node:addChild(s)
    end
    node:setPosition(700, 100 + 10 * c)
    return node
end

return MainScene

@weibobo
Copy link
Author

weibobo commented Dec 9, 2015

@xiaofeng11 , After running this test case, you will see two buttons on the sceen, click it you will see the time delta on screen.

  1. remove before use about 0.04s
  2. remove after use about 0.12s

@xiaofeng11
Copy link
Contributor

@weibobo I've test it with version 3.9, the issue is exist, I'll try to solved it, please give me some time

@weibobo
Copy link
Author

weibobo commented Dec 9, 2015

@xiaofeng11, thanks, seems ccui.imageview is easy to create this problem. if i use the cc.sprite, the time also increase, but much less.

@xiaofeng11
Copy link
Contributor

@weibobo I've do many profile test for this issue. The most cost operation beside drawing is in vector operation. And in the sample you provide, "oldlayer" hasn't init when program start, we notice first time whatever "add before" or "add after" operation will use slight more time, after analysis, we think the performance issue has follow reason:

  1. The first time add new layer use slight more time reason is at default vector size is not enough contain all nodes, so when add many nodes frequently, vector has been expand many times, this cause first add will get bad performance.
  2. "Remove after" operation old layer hasn't remove when new layer added, so this visit call need enum all sprite in old and new layer, it means this time need draw will do double operation, and when "oldlayer:removeFromParent()" been called, sprite in old layer is at middle of vector, vector can only give better performance when operation at head or tail. So here has a bad performance.

After discus with other members of our group, we think optimise cocos2dx can not give much help for these issue. For the first reason, try to init all sprite at start may is a choice. For the second we suggest deleter use less layer before create new layer.

@weibobo
Copy link
Author

weibobo commented Dec 10, 2015

@xiaofeng11, thanks, I still think there is a bug. we can skip the first time, and focus on the regular add and remove.
I have few questions:

  1. In "remove after", actually the new layer has not beened added yet. It's only been created.You can see the repo.
  2. In "remove after", the old layer has been removed before draw, so it will not affect "draw" . And the performance log actually did not count the draw time.
  3. oldlayer's parent only has a few children, how does it need so much time?

@weibobo
Copy link
Author

weibobo commented Dec 10, 2015

In our project, remove a layer need about 0.4-0.5s. It's too long for a mobile game.

@weibobo
Copy link
Author

weibobo commented Dec 10, 2015

Is there a vector include all the nodes in all layer? could u tell me which one?

@xiaofeng11
Copy link
Contributor

@weibobo removeFromParent is not only remove layer, but also will remove it's 300 hundred imageView child from autorelease pool, enum so many item from vector to release and then delete them from vector consume many cpu time.

All binding object created by create function is marked as auto-release object and will be add to auto release pool. You may refer CCAutoreleasePool.h & CCAutoreleasePool.cpp

@weibobo
Copy link
Author

weibobo commented Dec 10, 2015

@xiaofeng11, Looks like it's the auto release pool issue, in void Ref::release(). After remove this line, the time drops, thanks.
if (!poolManager->getCurrentPool()->isClearing() && poolManager->isObjectInPools(this))

@weibobo weibobo closed this as completed Dec 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants