----- Am 15. Aug 2019 um 8:26 schrieb Chris Johns chr...@rtems.org: > On 15/8/19 4:05 pm, Sebastian Huber wrote: >> ----- Am 15. Aug 2019 um 2:09 schrieb Chris Johns chr...@rtems.org: >> >>> On 14/8/19 3:19 pm, Sebastian Huber wrote: >>>> On 14/08/2019 03:57, Chris Johns wrote: >>>>> On 13/8/19 3:10 pm, Sebastian Huber wrote: >>>>>> sorry for the rush, >>>>> >>>>> Sorry for the delay, I have a client demo this week I am helping with. >>>>> >>>>>> but what do you think of this patch? >>>>> >>>>> Why not C++? The rtems-tools repo has strong support for C++ and it >>>>> brings a >>>>> range of benefits, for example no need to code call handlers at a low >>>>> level, >>>>> containers so no need for us to include and maintain trace/record/tree.h, >>>>> and >>>>> more. When I see us adding code like `tree.h` I have in the back of my >>>>> mind the >>>>> long term support issues it brings while a `std::map` is maintained for >>>>> us. >>>> >>>> Originally, the program should be able to filter live traces with about >>>> 20MiB/s. >>>> The std::map is simply too slow. >>> >>> It is difficult to comment without us heading into detail and I do not think >>> there is any value in doing that. >>> >>>> Boost.Intrusive would be an option (it is >>>> slower than tree.h in my tests too: https://github.com/sebhub/rb-bench). >>>> The >>>> tree.h is a copy from Newlib, so it doesn't introduce new maintenance >>>> issues. >>>> Anyway, for the CTF conversion the map is unnecessary and could be >>>> optimized >>>> away. We didn't had the time to do this in the GSoC project. >>> >>> So performance is not an issue here and the code's presence is historical? >> >> Ravindra needs this patch to integrate his work on top of it. I posted this >> patch on January and at this time using C was not an issue: >> >> https://lists.rtems.org/pipermail/devel/2019-January/024640.html >> > > Yes and I am sorry I did not raise this in that specific case. > >> His integration is now blocked because of something he didn't cause. > > Yes and I would to avoid there being a block. > >>> >>>>> GNU projects like gdb and gcc have moved to C++ and of course llvm is C++ >>>>> and >>>>> this is for good reason. I provide these examples in the hope this does >>>>> not >>>>> start a C vs C++ debate. >>>>> >>>>>> I would like to >>>>>> integrate the tracing work of the GSoC project and this patch is a >>>>>> blocking >>>>>> point. >>>>> >>>>> I understand. I am excited by the work that has been done here and what >>>>> you are >>>>> doing. It is taking all my will power to not read the ARM debug trace >>>>> section of >>>>> an ARM TRM as I think that hardware is ripe for integration with this >>>>> code base >>>>> and set of tools. A C++ framework in rtemstoolkit would be really helpful >>>>> if I >>>>> do as it would grow what we have rather than us collecting isolated C >>>>> programs. >>>>> >>>>> I also understand and appreciate the limited time we all have so I am >>>>> happy to >>>>> hear how you see the host side progressing over time and how it fits into >>>>> our >>>>> ecosystem. I would also like to hear what others think. >>>> >>>> I don't have a problem with C++ in general. However, I don't think that >>>> the use >>>> of C in this small program is a blocking point to integrate the GSoC work. >>>> This is work in progress anyway. This program only scratches the surface. >>> >>> What parts are to be added that depend on this tool? Maybe it would help if >>> this >>> is presented. >>> >>> Work in progress pieces are fine if there is a progression however I did >>> state >>> at the start of GSoC we currently use python and C++. I am weary of isolated >>> tools that become unclear if we need to keep or we can remove after a >>> period of >>> time. >> >> Sorry, but you should have stated that C++ is mandatory a bit earlier. > > I did ... > > https://lists.rtems.org/pipermail/devel/2019-May/025957.html > > There is C code in rtems-tools, ie elftools and bin2c and I am sure there will > be others. In the case of trace there is a history of C++ and I think this > work > is important enough that we consider the long term use cases. I have attempted > to avoid stating C++ is mandatory because that places us in a corner. > >> Now it is difficult to change. Ravindra did his work on top of a C program. >> We >> can easily convert it to C++, but only after the integration of Ravindra's >> work. The tool is already useful, it converts the RTEMS record stream into a >> CTF stream which can be read by Trace Compass to display the thread switches >> on >> multiple CPUs and the CPU utilization. > > Great. This is all I am asking for. I would prefer we resolve what happens > before we merge and we avoid a misunderstanding.
My approach would be to merge it as is. I can then simplify it and remove the tree.h. Currently, the tool merges record items streams per-CPU into one ordered record stream, then this merged stream is again split up into one CTF event stream per-CPU. The record-client.c and record-text.c are shared with RTEMS and should not be converted to C++. > >>> Tools added to rtems-tools and installed are public facing user interfaces >>> to >>> our users. By installing the RTEMS project is agreeing to provide and >>> support >>> the interface provided. I am fine with tools being add if they serve other >>> roles, for example as a means to test rtemstoolkit code, ie regression and >>> development test tools. It is unclear to me where this tool fits. >>> >>> Maybe we need an option to rtems-tools's configure such as --experimental >>> that >>> builds and installs tools that are a work in progress but are not a public >>> interface? >> >> Ok, sorry, I should have started with the integration planning of the GSoC >> work >> a bit earlier. > > Thanks, I am sure we can resolve this. > > Do you think --experimental is worth my time adding? This would mean we > refactor > the tool and then we remove the experimental status. It is a simple change, > add > the option and then add an if experimental to the install part of the waf > script. I don't think it is worth to add this switch. I will work on this program in August and September. _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
