Incorporating FrameRetrace upstream into ApiTrace
José Fonseca
jfonseca at vmware.com
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>
More information about the apitrace
mailing list