search

ManlyMarco / RuntimeUnityEditor

In-game inspector and debugging tools for applications made with Unity3D game engine
GNU General Public License v3.0
810 stars 99 forks source link

Aggregate button added #65

Closed takahiro0327 closed 9 months ago

takahiro0327 commented 9 months ago

Aggregate button added. It aggregates ticks and gcbytes of the same function. Please merge if you like.

aggregation

It was clear that DynamicBone was the slowest, but we wanted to be sure. I also wanted to make sure that there were not a large number of instances at the bottom of the list that would cause slowness.

takahiro0327 commented 9 months ago

After reviewing it, the key brackets were lame, so I changed them to columns. image

ManlyMarco commented 9 months ago

How does aggregation affect the order column ? Is the order counted from the 1st instance or is it random?

takahiro0327 commented 9 months ago

Kind of random. To be precise, the order in which they are added to _data is the oldest.

The first value in the sequence of GroupBy return values is used. (group.First()) This is the first value found in a sequence of _data.Values in ProfilerInfo belonging to a group.

The order of Dictionary.Values used in KKS is the order in which they were added. So it will return the numeric value of the oldest instance in the group. If the scene was just loaded, you might expect this to be close to the smallest value in the group.

We can still read the rough order, but should we make it so that the smallest value in a group is displayed?

This is unrelated, but added re-sorting when changing options.

takahiro0327 commented 9 months ago

Fixed a problem that the data is not aggregated when _needResort flag is OFF. Fixed to make order values more stable.

ManlyMarco commented 9 months ago

I think the smallest value should be used for the order.

takahiro0327 commented 9 months ago

Is it this way? What difference does it make?

image

ManlyMarco commented 9 months ago

I just took a look at the source and the current Max implementation should be fine. I'll test it later and merge if all is good.

takahiro0327 commented 9 months ago

Sorry if you have already tested it. I noticed a bug and will fix it.

takahiro0327 commented 9 months ago

Bug fixed. Past values were included in the aggregate.

ManlyMarco commented 9 months ago

Tested the latest commit. There appears to be an issue where the list stops updating after enabling and then disabling Aggregation. It sometimes resumes updating after closing and reopening the profiler window, but then the list appears to be corrupted. Unhooking and then re-hooking fixes the issue.

Normal image

After enabling aggregation image

After disabling aggregation (list no longer updates) image

After closing and reopening the profiler window (list should be exactly the same as in step 1 but the order values are different and sometimes it doesn't update) image

takahiro0327 commented 9 months ago

Thanks for the review. Fixed. How about this.

ManlyMarco commented 9 months ago

It updates correctly now. The only issue left that I can see is the "Ran" column shows the right toggle as off incorrectly when it should be on.

It might be best to remove the right toggle in Aggregate mode since some methods might run and others might not. Maybe replace the column with "any ran". Alternatively 3-state checkboxes could be used I suppose, but I'm not sure if IMGUI supports that.

takahiro0327 commented 9 months ago

Sorry, I didn't think that far ahead.

Remove right toggle when in aggregate mode.

Separate aggregate from executed and non-executed. The toggle on the left should now be accurate.

Is the SinceLastRun type of byte an intentional specification? Sometimes there were moments when it overflowed and things that were not working seemed to be working. I changed the type to uint for now.

takahiro0327 commented 9 months ago

Thanks too for the review and the useful tool! This profiler helped me fix my code which was slow.