Incorporating FrameRetrace upstream into ApiTrace

Wed May 1 22:19:57 UTC 2019

Hi Mark,

Sorry for the delay.  You caught me when I was on vacation, and I've been
busy catching up with things after coming back.

I haven't had time to play with FrameRetrace much.

First of all I want to reiterate some of my initials comments about
FrameRetrace a while back, when it was first announced, on
https://lists.freedesktop.org/archives/apitrace/2015-June/001087.html .

On one hand, it doesn't seem that FrameRetrace requires many changes to
existing code, per the output of `git diff --diff-filter=DM $(git
merge-base master frameretrace)..frameretrace` (seen
https://pastebin.com/E5WNNG8U ), which is good.   Still, this needs to be
cleaned, as a lot of these changes look spurious, and they should be
committed to apitrace master ahead of an eventual frameretrace merge.

But on the other hand, I still feel that several things were done
differently (and unnecessarily redone in cases.)  Most noticeably: a
completely separate GUI, with a completely different serialization format,
using a separate daemon instead of building upon the existing glretrace
slave processes.

Even if I can accept a completely separate GUI on apitrace source tree (not
ideal, because it will confuse users, and future contributors, but I might
come to make peace with it), I can't accept the server as it stands.  In
particular `daemon/glretrace_globals.cpp` needs to disappear somehow.  The
server needs to look more like the existing glretrace/d3dretrace/etc
(possibly it could even be merged into these), and not duplicate internal
code.

But honestly I feel it's too little too late.  Below you're focusing on use
cases, whereas my concerns are all but that -- I care about the
architecture and ease of maintenance. I don't doubt FrameRetrace is great
for users.  But if the goal is to merge, then the right architecture should
be chosen -- less duplication and more integration.

Plus fact you think you done all you planed to do is a big red flag to my
ears. I'm already treading water with my apitrace maintenance.  This will
be a butt load of code which I have little interest for.  It's all GL
specific  - a dying API (even if a slow death.)  A completely unknown code
base.   The core ideas are indeed interesting, but they could be more
easily achieved within the current components, as opposed to an wholesale
re-implementation.

Sorry, but I don't want to give false hopes: merging seems unlikely at this
stage...

Jose

PS: It's also surprising to see completely different indentation and code
style. I know that's easily fixable nowadays with tools, but to me is a
clear sign of how merging this code was a non-goal.  If the plan was to
merge, why not just get these things right off the bat?

On Tue, Apr 16, 2019 at 8:16 PM Mark Janes <mark.a.janes at intel.com> wrote:

> Hello José,
>
> I'm interested in getting the FrameRetrace UI upstream in the ApiTrace
> project.  The benefits of doing this would be:
>
>  - When ApiTrace changes the trace format, I have to rebase the
>    FrameRetrace branch.  This is a tedious and time consuming project.
>
>  - Upstream ApiTrace users would benefit from a far more responsive
>    and fully-featured application for investigating rendering and
>    performance problems.
>
>  - FrameRetrace would likely benefit from more users/collaborators.
>
> A possible *disadvantage* of upstreaming FrameRetrace would be
> increased overhead due to ApiTrace merge/review process.  Is there a
> way to incorporate FrameRetrace that wouldn't create additional work
> for you?
>
> I have implemented nearly all of the features that I anticipated for
> FrameRetrace.  The application is quite usable, and I have gotten
> feedback from Mesa devs and OpenGL developers that they have quickly
> investigated and solved problems with the tool.  OpenGL debugging
> features work without metrics, but Mesa developers have enabled
> hardware metrics for at least the following platforms:
>
>  - Intel (i965)
>  - AMD (Radeon)
>  - Broadcom (VC4)
>  - Qualcomm (Freedreno)
>  - Nvidia (Nouveau)
>
>    (caveat: I haven't done testing myself, but I fielded questions
>    from devs that enabled the platforms and heard that they got
>    metrics working).
>
> I have a bucket list of features to add or improve, but I am reluctant
> to spend time on them without feedback from users to indicate the
> value of the functionality.  I would eagerly invest time to make
> FrameRetrace more suitable for upstream.  Examples might include:
>
>  - Documentation/screenshots/demos
>  - Feedback from your review
>  - CMake changes in ApiTrace that I made to enable FrameRetrace (it's
>    probably not perfect)
>  - Visibility changes that I made to expose ApiTrace data structures
>  - Any bugs that end-users encounter with specific application traces.
>
> What is your opinion of FrameRetrace, and the possibility of
> incorporating it into your project?  Are there more features that you
> would like to see?  What information would help you evaluate the
> proposal?
>
> Thanks,
>
> -Mark
> _______________________________________________
> apitrace mailing list
> apitrace at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/apitrace
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/apitrace/attachments/20190501/ae4127e6/attachment.html>