Perf.printWasted prints newly inserted dom nodes in list as a waste of time

[slorber]

First of all, during my attempt to show you this bug, I found this weird behavior:

1. http://jsfiddle.net/kb3gN/4092/ does not show any time wasted (on user click action)
2. http://jsfiddle.net/kb3gN/4093/ does
   I don't know why one does show wasted time and not the other

On 2) I use shouldComponentUpdate=true, and we can see the time waste grows with the list size as well as the number of instances in the table.

So finally, to demonstrate my problem, see http://jsfiddle.net/kb3gN/4094/
I've implemented correctly shouldComponentUpdate and now each time I add a new element in the list, it is considered as a waste of time. My table always show 1 instance: the one that just got inserted.

I've had this problem on my app, when dealing with a paginated list (loading more items from the server as the user scrolls down). I don't think this newly inserted node should be printed in printWasted

[zpao]

@petehunt is on vacation right now but hopefully he'll have a chance to chime in when he gets back. @josh might be able to help out too if he's around.

[sophiebits]

Probably @joshduck. :)

[zpao]

Oops, sorry Josh! If you want to send some GH staff over to help debug though…

[sophiebits]

Curiously, this seems inconsistent if I leave the (third) page open and just watch it update:

[slorber]

Yes I've seen something like that too @spicyj ! but for my real app it seems to print the table constantly.
Maybe the inserted node is considered a waste but sometimes it works fine and it is not...

[jeffchan]

Running into same issue here #2105
Inconsistently in the reproduction, consistently in my own app as well.

[bobbyrenwick]

👍 having the same issue!

[koba04]

I suppose it's because setProps is batch update.
You should print in callback.

    component.setProps({state: state}, function() {
        React.addons.Perf.stop();
        React.addons.Perf.printWasted();
    });

The fixed fiddles are below.

• http://jsfiddle.net/koba04/5st9eaur/1/
• http://jsfiddle.net/koba04/0dhpegsh/

Unfortunately, totalTime is still incorrect...
#3436

[slorber]

thanks @koba04 will try that.

Also I linked a related question, can you take a look? #3611

[jaehunro]

Was this ever resolved? React.addons.Perf is still behaving similarly with newly inserted dom nodes being printed as wasted time. Also, @koba04 could I get some clarity on why the first fiddle prints wasted time but the second does not?

[slorber]

No this is not resolved as of 0.13 and I don't think either in 0.14

[sophiebits]

We haven't had a chance to look, no. Improving perf tooling is on our goals for 2015 but it won't happen before 0.14.

[koba04]

@jaehunro
the fiddles are different shouldComponentUpdate implementation.
the first fiddle is implemented as it returns always true.
the second fiddle is implemented as it returns true only when the contents are different.
As a result, the first one has many wasted times, the second one hasn't wasted time.

But I can't understand the second fiddle prints wasted time sometimes...

[jaehunro]

Thank you so much for the quick responses! I took a look through the source code to gain some understanding on how printWasted works and saw that wasted time is based on the time spent on 'clean' components. What decides whether components are clean/dirty is getUnchangedComponents in test/ReactDefaultPerfAnalysis.js which checks which component ids are contained in dirtyLeafIDs (the keys of measurement.writes).

However, newly inserted DOM nodes were being counted as clean because __recordWrite in test/ReactDefaultPerf.js was only recording the parent component id as a key in measurement.writes when new children were being inserted. I tried to work around this by keeping a record of created components by checking if the fnName in __recordWrite was INSERT_MARKUP. I added this record to measurement to be used in getUnchangedComponents to count newly created nodes as dirty.

Is my thinking on the right track or misguided?

[sophiebits]

Fixed by #4683.