04/05/2020 19:54, Jerin Jacob: > On Mon, May 4, 2020 at 11:10 PM David Marchand > > On Mon, May 4, 2020 at 7:19 PM Jerin Jacob <jerinjac...@gmail.com> wrote: > > > On Mon, May 4, 2020 at 10:38 PM David Marchand > > > > On Mon, May 4, 2020 at 4:39 PM Jerin Jacob <jerinjac...@gmail.com> > > > > wrote: > > > > > On Mon, May 4, 2020 at 7:34 PM David Marchand > > > > > <david.march...@redhat.com> wrote: > > > > > > On Mon, May 4, 2020 at 4:47 AM Jerin Jacob <jerinjac...@gmail.com> > > > > > > wrote: > > > > > > > On Mon, May 4, 2020 at 2:02 AM David Marchand > > > > > > > <david.march...@redhat.com> wrote: > > > > > > > > > > > > > > > > RTE_TRACE_POINT_DEFINE and RTE_TRACE_POINT_REGISTER must come > > > > > > > > in pairs. > > > > > > > > Merge them and let RTE_TRACE_POINT_REGISTER handle the > > > > > > > > constructor part. > > > > > > > > > > > > > > > > > > > > > Initially, I thought of doing the same. But, later I realized that > > > > > > > this largely grows the number of constructors been called. > > > > > > > I had concerns about the boot time of the application and/or > > > > > > > loading > > > > > > > the shared library, that the reason why spitting > > > > > > > as two so that constructor registers a burst of traces like > > > > > > > rte_log. > > > > > > > > > > > > I am a bit skeptical. > > > > > > In terms of cycles and looking at __rte_trace_point_register() > > > > > > (which > > > > > > calls malloc), the cost of calling multiple constructors instead of > > > > > > one is negligible. > > > > > > > > > > We will have a lot tracepoints latter, each one translates to the > > > > > constructor may not be a good > > > > > improvement. The scope is limited only to register function so IMO it > > > > > is okay to have split > > > > > just like rte_log. I don't see any reason why it has to be a different > > > > > than rte_log. > > > > > > > > What is similar to rte_log? > > > > There is neither RTE_LOG_REGISTER macro, nor two-steps declaration of > > > > dynamic logtypes. > > > > > > > > > Here is an example of rte_log registration. Which has _one_ > > > constructor and N number of > > > rte_log_register() underneath. > > > > rte_log is one thing, rte_trace is already different. > > > > There is _no macro_ in rte_log for registration. > > The reason being in that a rte_log logtype is a simple integer without > > any special declaration requiring a macro. > > I just wrapped in macro for convincing, but it has the same semantics. > global variable and API/macro to register. > > > > > > For tracepoints, we have a special two steps thing: the tracepoint > > handle must be derived from the tracepoint name. > > Then this handle must be registered. > > What I proposed is to make life easier for developers that want to add > > tracepoints and I suppose you noticed patch 1 of this series. > > To reduce the constructors. I don't want trace libraries to add lot of > constructors. > I don't think it simplifies a lot as the scope of only for registration. > > > > > > > > > > > One of the thought process is, we probably remove the constructor > > > > > scheme to all other with DPDK > > > > > and replace it with a more register scheme. In such a case, we can > > > > > skip calling the constructor all tother > > > > > when trace is disabled. > > > > > > > > Sorry, but I have a hard time understanding your point. > > > > Are you talking about application boot time? > > > > > > Yes. The optimization of application boottime time in case of static > > > binary and/or shared library(.so) load time. > > > > As Thomas mentioned, do you have numbers? > > No. But I know, it is obvious that current code is better in terms of > boot time than the proposed one. > I would like to not add a lot of constructor for trace as the FIRST > module in DPDK.
No, it is not obvious. The version from David looks simpler to use and to understand. Without any number, I consider usability (and maintenance) wins.
