From: Vincent T. <vt...@un...> - 2011-10-26 08:56:34
|
Hey Eina includes eina_inline_lock_posix.h on something else than Windows, hence pthread.h. _GNU_SOURCE is not defined. Suppose now that a user of Eina does this: #include <Eina.h> #include <pthread.h> The user will not have the possibility to features available with _GNU_SOURCE (like CPU_SET for example. I have that problem with Enesim), except by defining it just before including Eina.h. Which is not the best solution, I think. The problem, here, is that lock stuff is only inlined functions. The problem will be solved if they are in a source file. Maybe at the beginning, having these functions inlined was interesting because they were short. I'm not sure that keeping them inlined is really useful, now. Another solution would be to define _GNU_SOURCE before including pthread.h (maybe under some conditions). But is it a good solution too ? Honestly, I don't know what the best solution is. So if someone knows how to properly fix that problem... Vincent |
From: Cedric B. <ced...@fr...> - 2011-10-26 09:04:19
|
Hi, On Wed, Oct 26, 2011 at 10:55 AM, Vincent Torri <vt...@un...> wrote: > Eina includes eina_inline_lock_posix.h on something else than Windows, > hence pthread.h. _GNU_SOURCE is not defined. > > Suppose now that a user of Eina does this: > > #include <Eina.h> > #include <pthread.h> > > The user will not have the possibility to features available with > _GNU_SOURCE (like CPU_SET for example. I have that problem with Enesim), > except by defining it just before including Eina.h. Which is not the best > solution, I think. > > The problem, here, is that lock stuff is only inlined functions. The > problem will be solved if they are in a source file. Maybe at the > beginning, having these functions inlined was interesting because they > were short. I'm not sure that keeping them inlined is really useful, now. As from a performance point of view, it really matter to have them inlined or not. Function call does cost. > Another solution would be to define _GNU_SOURCE before including pthread.h > (maybe under some conditions). But is it a good solution too ? > > Honestly, I don't know what the best solution is. So if someone knows how > to properly fix that problem... I have always started to put libc header first if I need them directly and then include other library. This just solve this kind of issue. So I don't thing it's an issue to solve. -- Cedric BAIL |
From: Vincent T. <vt...@un...> - 2011-10-26 09:10:25
|
On Wed, 26 Oct 2011, Cedric BAIL wrote: > Hi, > > On Wed, Oct 26, 2011 at 10:55 AM, Vincent Torri <vt...@un...> wrote: >> Eina includes eina_inline_lock_posix.h on something else than Windows, >> hence pthread.h. _GNU_SOURCE is not defined. >> >> Suppose now that a user of Eina does this: >> >> #include <Eina.h> >> #include <pthread.h> >> >> The user will not have the possibility to features available with >> _GNU_SOURCE (like CPU_SET for example. I have that problem with Enesim), >> except by defining it just before including Eina.h. Which is not the best >> solution, I think. >> >> The problem, here, is that lock stuff is only inlined functions. The >> problem will be solved if they are in a source file. Maybe at the >> beginning, having these functions inlined was interesting because they >> were short. I'm not sure that keeping them inlined is really useful, now. > > As from a performance point of view, it really matter to have them > inlined or not. Function call does cost. > >> Another solution would be to define _GNU_SOURCE before including pthread.h >> (maybe under some conditions). But is it a good solution too ? >> >> Honestly, I don't know what the best solution is. So if someone knows how >> to properly fix that problem... > > I have always started to put libc header first if I need them directly > and then include other library. This just solve this kind of issue. So > I don't thing it's an issue to solve. well, if you think that everyone on earth must code like you... For me it is a bug. If you don't want to change anything, then documentation must describe that, and in addition, adding a #warning in eina_inline_posix.x if _GNU_SOURCE is not defined would be useful too. Vincent |
From: Vincent T. <vt...@un...> - 2011-10-26 09:13:31
|
On Wed, 26 Oct 2011, Cedric BAIL wrote: > Hi, > > On Wed, Oct 26, 2011 at 10:55 AM, Vincent Torri <vt...@un...> wrote: >> Eina includes eina_inline_lock_posix.h on something else than Windows, >> hence pthread.h. _GNU_SOURCE is not defined. >> >> Suppose now that a user of Eina does this: >> >> #include <Eina.h> >> #include <pthread.h> >> >> The user will not have the possibility to features available with >> _GNU_SOURCE (like CPU_SET for example. I have that problem with Enesim), >> except by defining it just before including Eina.h. Which is not the best >> solution, I think. >> >> The problem, here, is that lock stuff is only inlined functions. The >> problem will be solved if they are in a source file. Maybe at the >> beginning, having these functions inlined was interesting because they >> were short. I'm not sure that keeping them inlined is really useful, now. > > As from a performance point of view, it really matter to have them > inlined or not. Function call does cost. I know that, but i would like to have numbers, here, to verify it's worth having them inlined. Note that I'm talking about the posix part, not the 'void' or windows part. If your argument is : "no numbers are needed, it's faster", then why not defining all the functions inlined ? Vincent |
From: Cedric B. <ced...@fr...> - 2011-10-26 10:17:37
|
On Wed, Oct 26, 2011 at 11:13 AM, Vincent Torri <vt...@un...> wrote: > On Wed, 26 Oct 2011, Cedric BAIL wrote: >> On Wed, Oct 26, 2011 at 10:55 AM, Vincent Torri <vt...@un...> wrote: >>> Eina includes eina_inline_lock_posix.h on something else than Windows, >>> hence pthread.h. _GNU_SOURCE is not defined. >>> >>> Suppose now that a user of Eina does this: >>> >>> #include <Eina.h> >>> #include <pthread.h> >>> >>> The user will not have the possibility to features available with >>> _GNU_SOURCE (like CPU_SET for example. I have that problem with Enesim), >>> except by defining it just before including Eina.h. Which is not the best >>> solution, I think. >>> >>> The problem, here, is that lock stuff is only inlined functions. The >>> problem will be solved if they are in a source file. Maybe at the >>> beginning, having these functions inlined was interesting because they >>> were short. I'm not sure that keeping them inlined is really useful, now. >> >> As from a performance point of view, it really matter to have them >> inlined or not. Function call does cost. > > I know that, but i would like to have numbers, here, to verify it's worth > having them inlined. Note that I'm talking about the posix part, not the > 'void' or windows part. > > If your argument is : "no numbers are needed, it's faster", then why not > defining all the functions inlined ? I don't have access at the moment on machine where that does matter. But to put stuff into perspective, Eina_Magic check could cost around 10% of your time and it's just a function call with an if inside, much simpler that taking a lock. So I don't have number, but it's just way better to avoid the 10s instructions that are needed to do a function call. And why not inlining everything, that why we use static inline instead of a macro, gcc can choose to inline the function or not depending on all the cost implied by the function call. And we don't put all function inlined, because that would just increase the binary size and invalidate cache to much. So it is only a solution for very small function called very often. -- Cedric BAIL |
From: Cedric B. <ced...@fr...> - 2011-10-26 10:24:20
|
On Wed, Oct 26, 2011 at 11:10 AM, Vincent Torri <vt...@un...> wrote: > On Wed, 26 Oct 2011, Cedric BAIL wrote: >> On Wed, Oct 26, 2011 at 10:55 AM, Vincent Torri <vt...@un...> wrote: >>> Eina includes eina_inline_lock_posix.h on something else than Windows, >>> hence pthread.h. _GNU_SOURCE is not defined. >>> >>> Suppose now that a user of Eina does this: >>> >>> #include <Eina.h> >>> #include <pthread.h> >>> >>> The user will not have the possibility to features available with >>> _GNU_SOURCE (like CPU_SET for example. I have that problem with Enesim), >>> except by defining it just before including Eina.h. Which is not the best >>> solution, I think. >>> >>> The problem, here, is that lock stuff is only inlined functions. The >>> problem will be solved if they are in a source file. Maybe at the >>> beginning, having these functions inlined was interesting because they >>> were short. I'm not sure that keeping them inlined is really useful, now. >> >> As from a performance point of view, it really matter to have them >> inlined or not. Function call does cost. >> >>> Another solution would be to define _GNU_SOURCE before including pthread.h >>> (maybe under some conditions). But is it a good solution too ? >>> >>> Honestly, I don't know what the best solution is. So if someone knows how >>> to properly fix that problem... >> >> I have always started to put libc header first if I need them directly >> and then include other library. This just solve this kind of issue. So >> I don't thing it's an issue to solve. > > well, if you think that everyone on earth must code like you... Actually, what would you say to someone that put #ifdef HAVE_CONFIG_H at the end of the C file. That's bad idea and that's the same here. It's just sane to put config first, then include C library and then all headers in their dependencies order. Because there is some inline/define value in the libc and in any header that could directly affect any header/library that use it, meaning any C library. It is a dependence of it and it make sense to just respect dependencies order to avoid any issue. > For me it is a bug. If you don't want to change anything, then documentation > must describe that, and in addition, adding a #warning in eina_inline_posix.x > if _GNU_SOURCE is not defined would be useful too. If you want. -- Cedric BAIL |
From: Vincent T. <vt...@un...> - 2011-10-26 10:37:27
|
On Wed, 26 Oct 2011, Cedric BAIL wrote: > On Wed, Oct 26, 2011 at 11:10 AM, Vincent Torri <vt...@un...> wrote: >> On Wed, 26 Oct 2011, Cedric BAIL wrote: >>> On Wed, Oct 26, 2011 at 10:55 AM, Vincent Torri <vt...@un...> wrote: >>>> Eina includes eina_inline_lock_posix.h on something else than Windows, >>>> hence pthread.h. _GNU_SOURCE is not defined. >>>> >>>> Suppose now that a user of Eina does this: >>>> >>>> #include <Eina.h> >>>> #include <pthread.h> >>>> >>>> The user will not have the possibility to features available with >>>> _GNU_SOURCE (like CPU_SET for example. I have that problem with Enesim), >>>> except by defining it just before including Eina.h. Which is not the best >>>> solution, I think. >>>> >>>> The problem, here, is that lock stuff is only inlined functions. The >>>> problem will be solved if they are in a source file. Maybe at the >>>> beginning, having these functions inlined was interesting because they >>>> were short. I'm not sure that keeping them inlined is really useful, now. >>> >>> As from a performance point of view, it really matter to have them >>> inlined or not. Function call does cost. >>> >>>> Another solution would be to define _GNU_SOURCE before including pthread.h >>>> (maybe under some conditions). But is it a good solution too ? >>>> >>>> Honestly, I don't know what the best solution is. So if someone knows how >>>> to properly fix that problem... >>> >>> I have always started to put libc header first if I need them directly >>> and then include other library. This just solve this kind of issue. So >>> I don't thing it's an issue to solve. >> >> well, if you think that everyone on earth must code like you... > > Actually, what would you say to someone that put #ifdef HAVE_CONFIG_H > at the end of the C file. That's bad idea and that's the same here. > It's just sane to put config first, hell, just look at raster's use of headers : he puts everything in a _private.h (config.h too). If he wants to use Eina: #include Eina.h #include "exported_header.h" #include "***_private.h" and boum, it will fail with Eina if he wants to use CPU_SET. He will have to include config.h before Eina.h in a specific source file, while it's alreay included in his _private.h. Don't tell me how to use these headers. In case you don't remember, it's ME who moved all the inclusion of the headers from the *_private.h to the source file, to avoid such problems (especially on Windows where it's even more evil). And I still think that it is a bug. Vincent > then include C library and then > all headers in their dependencies order. Because there is some > inline/define value in the libc and in any header that could directly > affect any header/library that use it, meaning any C library. It is a > dependence of it and it make sense to just respect dependencies order > to avoid any issue. > >> For me it is a bug. If you don't want to change anything, then documentation >> must describe that, and in addition, adding a #warning in eina_inline_posix.x >> if _GNU_SOURCE is not defined would be useful too. > > If you want. > -- > Cedric BAIL > > ------------------------------------------------------------------------------ > The demand for IT networking professionals continues to grow, and the > demand for specialized networking skills is growing even more rapidly. > Take a complimentary Learning@Cisco Self-Assessment and learn > about Cisco certifications, training, and career opportunities. > http://p.sf.net/sfu/cisco-dev2dev > _______________________________________________ > enlightenment-devel mailing list > enl...@li... > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > |
From: Jorge L. Z. M. <jor...@gm...> - 2011-10-26 11:45:26
|
On Wed, Oct 26, 2011 at 12:37 PM, Vincent Torri <vt...@un...> wrote: > > > On Wed, 26 Oct 2011, Cedric BAIL wrote: > >> On Wed, Oct 26, 2011 at 11:10 AM, Vincent Torri <vt...@un...> wrote: >>> On Wed, 26 Oct 2011, Cedric BAIL wrote: >>>> On Wed, Oct 26, 2011 at 10:55 AM, Vincent Torri <vt...@un...> wrote: >>>>> Eina includes eina_inline_lock_posix.h on something else than Windows, >>>>> hence pthread.h. _GNU_SOURCE is not defined. >>>>> >>>>> Suppose now that a user of Eina does this: >>>>> >>>>> #include <Eina.h> >>>>> #include <pthread.h> >>>>> >>>>> The user will not have the possibility to features available with >>>>> _GNU_SOURCE (like CPU_SET for example. I have that problem with Enesim), >>>>> except by defining it just before including Eina.h. Which is not the best >>>>> solution, I think. >>>>> >>>>> The problem, here, is that lock stuff is only inlined functions. The >>>>> problem will be solved if they are in a source file. Maybe at the >>>>> beginning, having these functions inlined was interesting because they >>>>> were short. I'm not sure that keeping them inlined is really useful, now. >>>> >>>> As from a performance point of view, it really matter to have them >>>> inlined or not. Function call does cost. >>>> >>>>> Another solution would be to define _GNU_SOURCE before including pthread.h >>>>> (maybe under some conditions). But is it a good solution too ? >>>>> >>>>> Honestly, I don't know what the best solution is. So if someone knows how >>>>> to properly fix that problem... >>>> >>>> I have always started to put libc header first if I need them directly >>>> and then include other library. This just solve this kind of issue. So >>>> I don't thing it's an issue to solve. >>> >>> well, if you think that everyone on earth must code like you... >> >> Actually, what would you say to someone that put #ifdef HAVE_CONFIG_H >> at the end of the C file. That's bad idea and that's the same here. >> It's just sane to put config first, > > hell, just look at raster's use of headers : he puts everything in a > _private.h (config.h too). If he wants to use Eina: > > #include Eina.h > > #include "exported_header.h" > #include "***_private.h" > > and boum, it will fail with Eina if he wants to use CPU_SET. He will have > to include config.h before Eina.h in a specific source file, while it's > alreay included in his _private.h. > > Don't tell me how to use these headers. In case you don't remember, it's > ME who moved all the inclusion of the headers from the *_private.h to the > source file, to avoid such problems (especially on Windows where it's even > more evil). > > And I still think that it is a bug. Hi all, I agree with Vincent, but beside the header order issue of how should we put our own headers (libraries or apps that use eina) to make eina happy, there's a more critical problem. The main issue here is that a compiler flag that in theory is for our own library/app only might modify eina's behaviour and the other way around, that's no sense and must not happen in any circumstance. Regards > > Vincent > >> then include C library and then >> all headers in their dependencies order. Because there is some >> inline/define value in the libc and in any header that could directly >> affect any header/library that use it, meaning any C library. It is a >> dependence of it and it make sense to just respect dependencies order >> to avoid any issue. >> >>> For me it is a bug. If you don't want to change anything, then documentation >>> must describe that, and in addition, adding a #warning in eina_inline_posix.x >>> if _GNU_SOURCE is not defined would be useful too. >> >> If you want. >> -- >> Cedric BAIL >> >> ------------------------------------------------------------------------------ >> The demand for IT networking professionals continues to grow, and the >> demand for specialized networking skills is growing even more rapidly. >> Take a complimentary Learning@Cisco Self-Assessment and learn >> about Cisco certifications, training, and career opportunities. >> http://p.sf.net/sfu/cisco-dev2dev >> _______________________________________________ >> enlightenment-devel mailing list >> enl...@li... >> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel >> >> > > ------------------------------------------------------------------------------ > The demand for IT networking professionals continues to grow, and the > demand for specialized networking skills is growing even more rapidly. > Take a complimentary Learning@Cisco Self-Assessment and learn > about Cisco certifications, training, and career opportunities. > http://p.sf.net/sfu/cisco-dev2dev > _______________________________________________ > enlightenment-devel mailing list > enl...@li... > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > |
From: Vincent T. <vt...@un...> - 2011-10-26 11:48:38
|
On Wed, 26 Oct 2011, Cedric BAIL wrote: > On Wed, Oct 26, 2011 at 11:13 AM, Vincent Torri <vt...@un...> wrote: >> On Wed, 26 Oct 2011, Cedric BAIL wrote: >>> On Wed, Oct 26, 2011 at 10:55 AM, Vincent Torri <vt...@un...> wrote: >>>> Eina includes eina_inline_lock_posix.h on something else than Windows, >>>> hence pthread.h. _GNU_SOURCE is not defined. >>>> >>>> Suppose now that a user of Eina does this: >>>> >>>> #include <Eina.h> >>>> #include <pthread.h> >>>> >>>> The user will not have the possibility to features available with >>>> _GNU_SOURCE (like CPU_SET for example. I have that problem with Enesim), >>>> except by defining it just before including Eina.h. Which is not the best >>>> solution, I think. >>>> >>>> The problem, here, is that lock stuff is only inlined functions. The >>>> problem will be solved if they are in a source file. Maybe at the >>>> beginning, having these functions inlined was interesting because they >>>> were short. I'm not sure that keeping them inlined is really useful, now. >>> >>> As from a performance point of view, it really matter to have them >>> inlined or not. Function call does cost. >> >> I know that, but i would like to have numbers, here, to verify it's worth >> having them inlined. Note that I'm talking about the posix part, not the >> 'void' or windows part. >> >> If your argument is : "no numbers are needed, it's faster", then why not >> defining all the functions inlined ? > > I don't have access at the moment on machine where that does matter. > But to put stuff into perspective, Eina_Magic check could cost around > 10% of your time and it's just a function call with an if inside, much > simpler that taking a lock. So I don't have number, but it's just way > better to avoid the 10s instructions that are needed to do a function > call. > And why not inlining everything, that was a sarcasm... > that why we use static inline > instead of a macro, gcc can choose to inline the function or not > depending on all the cost implied by the function call. And we don't > put all function inlined, because that would just increase the binary > size and invalidate cache to much. So it is only a solution for very > small function called very often. Look at the function eina_lock_take() in the posix file : 67 lines (with the defines). Do you call that a small function ? And I perfectly remember you telling to use **static** inline to force gcc to inline the function. Now you're saying that gcc will sometimes inline it, sometimes not ? You're contradicting yourself. > -- > Cedric BAIL > > ------------------------------------------------------------------------------ > The demand for IT networking professionals continues to grow, and the > demand for specialized networking skills is growing even more rapidly. > Take a complimentary Learning@Cisco Self-Assessment and learn > about Cisco certifications, training, and career opportunities. > http://p.sf.net/sfu/cisco-dev2dev > _______________________________________________ > enlightenment-devel mailing list > enl...@li... > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > |
From: Cedric B. <ced...@fr...> - 2011-10-26 12:29:42
|
On Wed, Oct 26, 2011 at 1:48 PM, Vincent Torri <vt...@un...> wrote: > On Wed, 26 Oct 2011, Cedric BAIL wrote: >> On Wed, Oct 26, 2011 at 11:13 AM, Vincent Torri <vt...@un...> wrote: >>> On Wed, 26 Oct 2011, Cedric BAIL wrote: >>>> On Wed, Oct 26, 2011 at 10:55 AM, Vincent Torri <vt...@un...> wrote: >>>>> Eina includes eina_inline_lock_posix.h on something else than Windows, >>>>> hence pthread.h. _GNU_SOURCE is not defined. >>>>> >>>>> Suppose now that a user of Eina does this: >>>>> >>>>> #include <Eina.h> >>>>> #include <pthread.h> >>>>> >>>>> The user will not have the possibility to features available with >>>>> _GNU_SOURCE (like CPU_SET for example. I have that problem with Enesim), >>>>> except by defining it just before including Eina.h. Which is not the best >>>>> solution, I think. >>>>> >>>>> The problem, here, is that lock stuff is only inlined functions. The >>>>> problem will be solved if they are in a source file. Maybe at the >>>>> beginning, having these functions inlined was interesting because they >>>>> were short. I'm not sure that keeping them inlined is really useful, now. >>>> >>>> As from a performance point of view, it really matter to have them >>>> inlined or not. Function call does cost. >>> >>> I know that, but i would like to have numbers, here, to verify it's worth >>> having them inlined. Note that I'm talking about the posix part, not the >>> 'void' or windows part. >>> >>> If your argument is : "no numbers are needed, it's faster", then why not >>> defining all the functions inlined ? >> >> I don't have access at the moment on machine where that does matter. >> But to put stuff into perspective, Eina_Magic check could cost around >> 10% of your time and it's just a function call with an if inside, much >> simpler that taking a lock. So I don't have number, but it's just way >> better to avoid the 10s instructions that are needed to do a function >> call. >> And why not inlining everything, > > that was a sarcasm... > >> that why we use static inline >> instead of a macro, gcc can choose to inline the function or not >> depending on all the cost implied by the function call. And we don't >> put all function inlined, because that would just increase the binary >> size and invalidate cache to much. So it is only a solution for very >> small function called very often. > > Look at the function eina_lock_take() in the posix file : 67 lines (with > the defines). Do you call that a small function ? And I perfectly remember > you telling to use **static** inline to force gcc to inline the function. > Now you're saying that gcc will sometimes inline it, sometimes not ? > You're contradicting yourself. No I am not, the static is here to prevent a clash between symbol. That's all it is saying and it will never force a function to be inlined. It just make it possible to the compiler to do so if it makes sense. I told to put static, because inline doesn't tell anything about the symbol visibility and that would be an issue. -- Cedric BAIL |
From: Youness A. <kak...@ka...> - 2011-10-28 12:14:07
|
I agree with vincent, and I think that there shouldn't be a "define it before including eina.h" implicit rule.. while it's not so bad, and it would work, it's not the right way to do it. these functions shouldn't be inlined I think, they should be a define, and for the posix functions, they are way too big to be inline, they would dramatically increase the generated code, especially if you lock/unlock everywhere. As for performance, give us hard numbers! I don't think it would affect performance for such function calls. So in my opinion, make them defines, and have the posix versions as actual functions that the defines call (so for non posix with smaller functions, you don't need an actual call). On Wed, Oct 26, 2011 at 8:29 AM, Cedric BAIL <ced...@fr...> wrote: > On Wed, Oct 26, 2011 at 1:48 PM, Vincent Torri <vt...@un...> > wrote: > > On Wed, 26 Oct 2011, Cedric BAIL wrote: > >> On Wed, Oct 26, 2011 at 11:13 AM, Vincent Torri <vt...@un...> > wrote: > >>> On Wed, 26 Oct 2011, Cedric BAIL wrote: > >>>> On Wed, Oct 26, 2011 at 10:55 AM, Vincent Torri <vt...@un...> > wrote: > >>>>> Eina includes eina_inline_lock_posix.h on something else than > Windows, > >>>>> hence pthread.h. _GNU_SOURCE is not defined. > >>>>> > >>>>> Suppose now that a user of Eina does this: > >>>>> > >>>>> #include <Eina.h> > >>>>> #include <pthread.h> > >>>>> > >>>>> The user will not have the possibility to features available with > >>>>> _GNU_SOURCE (like CPU_SET for example. I have that problem with > Enesim), > >>>>> except by defining it just before including Eina.h. Which is not the > best > >>>>> solution, I think. > >>>>> > >>>>> The problem, here, is that lock stuff is only inlined functions. The > >>>>> problem will be solved if they are in a source file. Maybe at the > >>>>> beginning, having these functions inlined was interesting because > they > >>>>> were short. I'm not sure that keeping them inlined is really useful, > now. > >>>> > >>>> As from a performance point of view, it really matter to have them > >>>> inlined or not. Function call does cost. > >>> > >>> I know that, but i would like to have numbers, here, to verify it's > worth > >>> having them inlined. Note that I'm talking about the posix part, not > the > >>> 'void' or windows part. > >>> > >>> If your argument is : "no numbers are needed, it's faster", then why > not > >>> defining all the functions inlined ? > >> > >> I don't have access at the moment on machine where that does matter. > >> But to put stuff into perspective, Eina_Magic check could cost around > >> 10% of your time and it's just a function call with an if inside, much > >> simpler that taking a lock. So I don't have number, but it's just way > >> better to avoid the 10s instructions that are needed to do a function > >> call. > >> And why not inlining everything, > > > > that was a sarcasm... > > > >> that why we use static inline > >> instead of a macro, gcc can choose to inline the function or not > >> depending on all the cost implied by the function call. And we don't > >> put all function inlined, because that would just increase the binary > >> size and invalidate cache to much. So it is only a solution for very > >> small function called very often. > > > > Look at the function eina_lock_take() in the posix file : 67 lines (with > > the defines). Do you call that a small function ? And I perfectly > remember > > you telling to use **static** inline to force gcc to inline the function. > > Now you're saying that gcc will sometimes inline it, sometimes not ? > > You're contradicting yourself. > > No I am not, the static is here to prevent a clash between symbol. > That's all it is saying and it will never force a function to be > inlined. It just make it possible to the compiler to do so if it makes > sense. I told to put static, because inline doesn't tell anything > about the symbol visibility and that would be an issue. > -- > Cedric BAIL > > > ------------------------------------------------------------------------------ > The demand for IT networking professionals continues to grow, and the > demand for specialized networking skills is growing even more rapidly. > Take a complimentary Learning@Cisco Self-Assessment and learn > about Cisco certifications, training, and career opportunities. > http://p.sf.net/sfu/cisco-dev2dev > _______________________________________________ > enlightenment-devel mailing list > enl...@li... > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > |
From: Cedric B. <ced...@fr...> - 2011-10-28 14:14:00
|
On Fri, Oct 28, 2011 at 2:13 PM, Youness Alaoui <kak...@ka...> wrote: > I agree with vincent, and I think that there shouldn't be a "define it > before including eina.h" implicit rule.. while it's not so bad, and it would > work, it's not the right way to do it. > these functions shouldn't be inlined I think, they should be a define, and Using define instead of inline has a lot issue. First it doesn't let the compiler when it's better or not to inline, it will always inline. Second it doesn't do any type checking so you can break stuff much more easily. And generally inline is much more easy to read than define. > for the posix functions, they are way too big to be inline, they would > dramatically increase the generated code, especially if you lock/unlock > everywhere. I actually don't understand what point your are trying to make here. > As for performance, give us hard numbers! I don't think it would > affect performance for such function calls. So in my opinion, make them > defines, and have the posix versions as actual functions that the defines > call (so for non posix with smaller functions, you don't need an actual > call). What does matter is call to small function, because the overhead of setting up the stack, saving register, jumping to the actual function to call and then coming by restoring register and the stack does introduce a cost. Something you will not see on your high end laptop, but on embedded device. And the inline function are function that are typically in the hot path of our stack, so function call really do matter. And moving to #define would not solve the issue Vincent is pointing as far as I understand it. > On Wed, Oct 26, 2011 at 8:29 AM, Cedric BAIL <ced...@fr...> wrote: > >> On Wed, Oct 26, 2011 at 1:48 PM, Vincent Torri <vt...@un...> >> wrote: >> > On Wed, 26 Oct 2011, Cedric BAIL wrote: >> >> On Wed, Oct 26, 2011 at 11:13 AM, Vincent Torri <vt...@un...> >> wrote: >> >>> On Wed, 26 Oct 2011, Cedric BAIL wrote: >> >>>> On Wed, Oct 26, 2011 at 10:55 AM, Vincent Torri <vt...@un...> >> wrote: >> >>>>> Eina includes eina_inline_lock_posix.h on something else than >> Windows, >> >>>>> hence pthread.h. _GNU_SOURCE is not defined. >> >>>>> >> >>>>> Suppose now that a user of Eina does this: >> >>>>> >> >>>>> #include <Eina.h> >> >>>>> #include <pthread.h> >> >>>>> >> >>>>> The user will not have the possibility to features available with >> >>>>> _GNU_SOURCE (like CPU_SET for example. I have that problem with >> Enesim), >> >>>>> except by defining it just before including Eina.h. Which is not the >> best >> >>>>> solution, I think. >> >>>>> >> >>>>> The problem, here, is that lock stuff is only inlined functions. The >> >>>>> problem will be solved if they are in a source file. Maybe at the >> >>>>> beginning, having these functions inlined was interesting because >> they >> >>>>> were short. I'm not sure that keeping them inlined is really useful, >> now. >> >>>> >> >>>> As from a performance point of view, it really matter to have them >> >>>> inlined or not. Function call does cost. >> >>> >> >>> I know that, but i would like to have numbers, here, to verify it's >> worth >> >>> having them inlined. Note that I'm talking about the posix part, not >> the >> >>> 'void' or windows part. >> >>> >> >>> If your argument is : "no numbers are needed, it's faster", then why >> not >> >>> defining all the functions inlined ? >> >> >> >> I don't have access at the moment on machine where that does matter. >> >> But to put stuff into perspective, Eina_Magic check could cost around >> >> 10% of your time and it's just a function call with an if inside, much >> >> simpler that taking a lock. So I don't have number, but it's just way >> >> better to avoid the 10s instructions that are needed to do a function >> >> call. >> >> And why not inlining everything, >> > >> > that was a sarcasm... >> > >> >> that why we use static inline >> >> instead of a macro, gcc can choose to inline the function or not >> >> depending on all the cost implied by the function call. And we don't >> >> put all function inlined, because that would just increase the binary >> >> size and invalidate cache to much. So it is only a solution for very >> >> small function called very often. >> > >> > Look at the function eina_lock_take() in the posix file : 67 lines (with >> > the defines). Do you call that a small function ? And I perfectly >> remember >> > you telling to use **static** inline to force gcc to inline the function. >> > Now you're saying that gcc will sometimes inline it, sometimes not ? >> > You're contradicting yourself. >> >> No I am not, the static is here to prevent a clash between symbol. >> That's all it is saying and it will never force a function to be >> inlined. It just make it possible to the compiler to do so if it makes >> sense. I told to put static, because inline doesn't tell anything >> about the symbol visibility and that would be an issue. >> -- >> Cedric BAIL >> >> >> ------------------------------------------------------------------------------ >> The demand for IT networking professionals continues to grow, and the >> demand for specialized networking skills is growing even more rapidly. >> Take a complimentary Learning@Cisco Self-Assessment and learn >> about Cisco certifications, training, and career opportunities. >> http://p.sf.net/sfu/cisco-dev2dev >> _______________________________________________ >> enlightenment-devel mailing list >> enl...@li... >> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel >> > ------------------------------------------------------------------------------ > The demand for IT networking professionals continues to grow, and the > demand for specialized networking skills is growing even more rapidly. > Take a complimentary Learning@Cisco Self-Assessment and learn > about Cisco certifications, training, and career opportunities. > http://p.sf.net/sfu/cisco-dev2dev > _______________________________________________ > enlightenment-devel mailing list > enl...@li... > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > -- Cedric BAIL |
From: Vincent T. <vt...@un...> - 2011-10-28 14:45:56
|
On Fri, 28 Oct 2011, Cedric BAIL wrote: > On Fri, Oct 28, 2011 at 2:13 PM, Youness Alaoui > <kak...@ka...> wrote: >> I agree with vincent, and I think that there shouldn't be a "define it >> before including eina.h" implicit rule.. while it's not so bad, and it would >> work, it's not the right way to do it. >> these functions shouldn't be inlined I think, they should be a define, and > > Using define instead of inline has a lot issue. First it doesn't let > the compiler when it's better or not to inline, it will always inline. > Second it doesn't do any type checking so you can break stuff much > more easily. And generally inline is much more easy to read than > define. > >> for the posix functions, they are way too big to be inline, they would >> dramatically increase the generated code, especially if you lock/unlock >> everywhere. > > I actually don't understand what point your are trying to make here. > >> As for performance, give us hard numbers! I don't think it would >> affect performance for such function calls. So in my opinion, make them >> defines, and have the posix versions as actual functions that the defines >> call (so for non posix with smaller functions, you don't need an actual >> call). > > What does matter is call to small function, because the overhead of > setting up the stack, saving register, jumping to the actual function > to call and then coming by restoring register and the stack does > introduce a cost. Something you will not see on your high end laptop, > but on embedded device. And the inline function are function that are > typically in the hot path of our stack, so function call really do > matter. And moving to #define would not solve the issue Vincent is > pointing as far as I understand it. indeed. So, with the argument of Jorge, there are 2 problems. Vincent |
From: Cedric B. <ced...@fr...> - 2011-10-29 17:50:12
|
On Fri, Oct 28, 2011 at 4:45 PM, Vincent Torri <vt...@un...> wrote: > On Fri, 28 Oct 2011, Cedric BAIL wrote: >> On Fri, Oct 28, 2011 at 2:13 PM, Youness Alaoui >> <kak...@ka...> wrote: >>> I agree with vincent, and I think that there shouldn't be a "define it >>> before including eina.h" implicit rule.. while it's not so bad, and it would >>> work, it's not the right way to do it. >>> these functions shouldn't be inlined I think, they should be a define, and >> >> Using define instead of inline has a lot issue. First it doesn't let >> the compiler when it's better or not to inline, it will always inline. >> Second it doesn't do any type checking so you can break stuff much >> more easily. And generally inline is much more easy to read than >> define. >> >>> for the posix functions, they are way too big to be inline, they would >>> dramatically increase the generated code, especially if you lock/unlock >>> everywhere. >> >> I actually don't understand what point your are trying to make here. >> >>> As for performance, give us hard numbers! I don't think it would >>> affect performance for such function calls. So in my opinion, make them >>> defines, and have the posix versions as actual functions that the defines >>> call (so for non posix with smaller functions, you don't need an actual >>> call). >> >> What does matter is call to small function, because the overhead of >> setting up the stack, saving register, jumping to the actual function >> to call and then coming by restoring register and the stack does >> introduce a cost. Something you will not see on your high end laptop, >> but on embedded device. And the inline function are function that are >> typically in the hot path of our stack, so function call really do >> matter. And moving to #define would not solve the issue Vincent is >> pointing as far as I understand it. > > indeed. So, with the argument of Jorge, there are 2 problems. No, there is no such problem. The compiler flag will not change any behaviour in eina library or I really missed something and please show me example code. And as for giving header random order support I really don't think it's important and worth it. It's just good practice to respect dependencies and don't rely on the hope that something magic will happen. -- Cedric BAIL |
From: Vincent T. <vt...@un...> - 2011-10-29 18:09:02
|
On Sat, 29 Oct 2011, Cedric BAIL wrote: > On Fri, Oct 28, 2011 at 4:45 PM, Vincent Torri <vt...@un...> wrote: >> On Fri, 28 Oct 2011, Cedric BAIL wrote: >>> On Fri, Oct 28, 2011 at 2:13 PM, Youness Alaoui >>> <kak...@ka...> wrote: >>>> I agree with vincent, and I think that there shouldn't be a "define it >>>> before including eina.h" implicit rule.. while it's not so bad, and it would >>>> work, it's not the right way to do it. >>>> these functions shouldn't be inlined I think, they should be a define, and >>> >>> Using define instead of inline has a lot issue. First it doesn't let >>> the compiler when it's better or not to inline, it will always inline. >>> Second it doesn't do any type checking so you can break stuff much >>> more easily. And generally inline is much more easy to read than >>> define. >>> >>>> for the posix functions, they are way too big to be inline, they would >>>> dramatically increase the generated code, especially if you lock/unlock >>>> everywhere. >>> >>> I actually don't understand what point your are trying to make here. >>> >>>> As for performance, give us hard numbers! I don't think it would >>>> affect performance for such function calls. So in my opinion, make them >>>> defines, and have the posix versions as actual functions that the defines >>>> call (so for non posix with smaller functions, you don't need an actual >>>> call). >>> >>> What does matter is call to small function, because the overhead of >>> setting up the stack, saving register, jumping to the actual function >>> to call and then coming by restoring register and the stack does >>> introduce a cost. Something you will not see on your high end laptop, >>> but on embedded device. And the inline function are function that are >>> typically in the hot path of our stack, so function call really do >>> matter. And moving to #define would not solve the issue Vincent is >>> pointing as far as I understand it. >> >> indeed. So, with the argument of Jorge, there are 2 problems. > > No, there is no such problem. The compiler flag will not change any > behaviour in eina library or I really missed something and please show > me example code. And as for giving header random order support I > really don't think it's important and worth it. did you read my mail about the example with raster's habit of putting everything in a private header ? I mean, really read it ? You didn't give any answer to it... Vincent > It's just good > practice to respect dependencies and don't rely on the hope that > something magic will happen. > -- > Cedric BAIL > > ------------------------------------------------------------------------------ > Get your Android app more play: Bring it to the BlackBerry PlayBook > in minutes. BlackBerry App World™ now supports Android™ Apps > for the BlackBerry® PlayBook™. Discover just how easy and simple > it is! http://p.sf.net/sfu/android-dev2dev > _______________________________________________ > enlightenment-devel mailing list > enl...@li... > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > |
From: Cedric B. <ced...@fr...> - 2011-10-31 15:40:13
|
On Sat, Oct 29, 2011 at 8:08 PM, Vincent Torri <vt...@un...> wrote: > On Sat, 29 Oct 2011, Cedric BAIL wrote: >> On Fri, Oct 28, 2011 at 4:45 PM, Vincent Torri <vt...@un...> wrote: >>> On Fri, 28 Oct 2011, Cedric BAIL wrote: >>>> On Fri, Oct 28, 2011 at 2:13 PM, Youness Alaoui >>>> <kak...@ka...> wrote: >>>>> I agree with vincent, and I think that there shouldn't be a "define it >>>>> before including eina.h" implicit rule.. while it's not so bad, and it would >>>>> work, it's not the right way to do it. >>>>> these functions shouldn't be inlined I think, they should be a define, and >>>> >>>> Using define instead of inline has a lot issue. First it doesn't let >>>> the compiler when it's better or not to inline, it will always inline. >>>> Second it doesn't do any type checking so you can break stuff much >>>> more easily. And generally inline is much more easy to read than >>>> define. >>>> >>>>> for the posix functions, they are way too big to be inline, they would >>>>> dramatically increase the generated code, especially if you lock/unlock >>>>> everywhere. >>>> >>>> I actually don't understand what point your are trying to make here. >>>> >>>>> As for performance, give us hard numbers! I don't think it would >>>>> affect performance for such function calls. So in my opinion, make them >>>>> defines, and have the posix versions as actual functions that the defines >>>>> call (so for non posix with smaller functions, you don't need an actual >>>>> call). >>>> >>>> What does matter is call to small function, because the overhead of >>>> setting up the stack, saving register, jumping to the actual function >>>> to call and then coming by restoring register and the stack does >>>> introduce a cost. Something you will not see on your high end laptop, >>>> but on embedded device. And the inline function are function that are >>>> typically in the hot path of our stack, so function call really do >>>> matter. And moving to #define would not solve the issue Vincent is >>>> pointing as far as I understand it. >>> >>> indeed. So, with the argument of Jorge, there are 2 problems. >> >> No, there is no such problem. The compiler flag will not change any >> behaviour in eina library or I really missed something and please show >> me example code. And as for giving header random order support I >> really don't think it's important and worth it. > > did you read my mail about the example with raster's habit of putting > everything in a private header ? I mean, really read it ? You didn't give > any answer to it... Yes, I did. And I still don't konw how to answer it, because I just agree with your point. I agree that's a bad habit, that it should be fixed and that your fix were right. And I think every one did as in so many year nobody reverted them. So I have nothing to argue with you about your argument at all, but you still see that as a bug, despite your argument. So I really don't know what to say or argue, as I just agree with your point. -- Cedric BAIL |
From: Carsten H. (T. R. <ra...@ra...> - 2011-11-01 00:00:15
|
On Wed, 26 Oct 2011 12:37:12 +0200 (CEST) Vincent Torri <vt...@un...> said: > > > On Wed, 26 Oct 2011, Cedric BAIL wrote: > > > On Wed, Oct 26, 2011 at 11:10 AM, Vincent Torri <vt...@un...> wrote: > >> On Wed, 26 Oct 2011, Cedric BAIL wrote: > >>> On Wed, Oct 26, 2011 at 10:55 AM, Vincent Torri <vt...@un...> > >>> wrote: > >>>> Eina includes eina_inline_lock_posix.h on something else than Windows, > >>>> hence pthread.h. _GNU_SOURCE is not defined. > >>>> > >>>> Suppose now that a user of Eina does this: > >>>> > >>>> #include <Eina.h> > >>>> #include <pthread.h> > >>>> > >>>> The user will not have the possibility to features available with > >>>> _GNU_SOURCE (like CPU_SET for example. I have that problem with Enesim), > >>>> except by defining it just before including Eina.h. Which is not the best > >>>> solution, I think. > >>>> > >>>> The problem, here, is that lock stuff is only inlined functions. The > >>>> problem will be solved if they are in a source file. Maybe at the > >>>> beginning, having these functions inlined was interesting because they > >>>> were short. I'm not sure that keeping them inlined is really useful, now. > >>> > >>> As from a performance point of view, it really matter to have them > >>> inlined or not. Function call does cost. > >>> > >>>> Another solution would be to define _GNU_SOURCE before including > >>>> pthread.h (maybe under some conditions). But is it a good solution too ? > >>>> > >>>> Honestly, I don't know what the best solution is. So if someone knows how > >>>> to properly fix that problem... > >>> > >>> I have always started to put libc header first if I need them directly > >>> and then include other library. This just solve this kind of issue. So > >>> I don't thing it's an issue to solve. > >> > >> well, if you think that everyone on earth must code like you... > > > > Actually, what would you say to someone that put #ifdef HAVE_CONFIG_H > > at the end of the C file. That's bad idea and that's the same here. > > It's just sane to put config first, > > hell, just look at raster's use of headers : he puts everything in a > _private.h (config.h too). If he wants to use Eina: > > #include Eina.h > > #include "exported_header.h" > #include "***_private.h" > > and boum, it will fail with Eina if he wants to use CPU_SET. He will have > to include config.h before Eina.h in a specific source file, while it's > alreay included in his _private.h. it's like that because no efl headers ever BEFORE relied on the "hosts" config.h - in fact they NEVER EVER EVER should do thbis. the header from a lib should provide the same features always -r egardless of what is in the apps config.h. sure - the app CAN try and disable features by playing with #define and #undef games but it should not accidentally happen - ie all #ifdefs should be namespaced OR be compiler or architecture or platform specific. relying in #define __GNU_SOURCE or other friends to be defined by the app is bad. > Don't tell me how to use these headers. In case you don't remember, it's > ME who moved all the inclusion of the headers from the *_private.h to the > source file, to avoid such problems (especially on Windows where it's even > more evil). that's a REAL pain in the arse as then the top of every file has the same set of 2, 5, 10, 20 header includes. it's maintenance hell. i put them all into the same common/private header and even if not all are used in each file, it provides a global scope for the app/lib/project that you don't have to keep re-figuring-out per file. you use eet* in file a then later need it in file b but find Eet.h isn't included so u have to go fix it again when it can just be a common include. > And I still think that it is a bug. > > Vincent > > > then include C library and then > > all headers in their dependencies order. Because there is some > > inline/define value in the libc and in any header that could directly > > affect any header/library that use it, meaning any C library. It is a > > dependence of it and it make sense to just respect dependencies order > > to avoid any issue. > > > >> For me it is a bug. If you don't want to change anything, then > >> documentation must describe that, and in addition, adding a #warning in > >> eina_inline_posix.x if _GNU_SOURCE is not defined would be useful too. > > > > If you want. > > -- > > Cedric BAIL > > > > ------------------------------------------------------------------------------ > > The demand for IT networking professionals continues to grow, and the > > demand for specialized networking skills is growing even more rapidly. > > Take a complimentary Learning@Cisco Self-Assessment and learn > > about Cisco certifications, training, and career opportunities. > > http://p.sf.net/sfu/cisco-dev2dev > > _______________________________________________ > > enlightenment-devel mailing list > > enl...@li... > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > > > > > ------------------------------------------------------------------------------ > The demand for IT networking professionals continues to grow, and the > demand for specialized networking skills is growing even more rapidly. > Take a complimentary Learning@Cisco Self-Assessment and learn > about Cisco certifications, training, and career opportunities. > http://p.sf.net/sfu/cisco-dev2dev > _______________________________________________ > enlightenment-devel mailing list > enl...@li... > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > -- ------------- Codito, ergo sum - "I code, therefore I am" -------------- The Rasterman (Carsten Haitzler) ra...@ra... |
From: Vincent T. <vt...@un...> - 2011-11-01 00:23:16
|
On Tue, 1 Nov 2011, Carsten Haitzler (The Rasterman) wrote: > On Wed, 26 Oct 2011 12:37:12 +0200 (CEST) Vincent Torri <vt...@un...> > said: > >> >> >> On Wed, 26 Oct 2011, Cedric BAIL wrote: >> >>> On Wed, Oct 26, 2011 at 11:10 AM, Vincent Torri <vt...@un...> wrote: >>>> On Wed, 26 Oct 2011, Cedric BAIL wrote: >>>>> On Wed, Oct 26, 2011 at 10:55 AM, Vincent Torri <vt...@un...> >>>>> wrote: >>>>>> Eina includes eina_inline_lock_posix.h on something else than Windows, >>>>>> hence pthread.h. _GNU_SOURCE is not defined. >>>>>> >>>>>> Suppose now that a user of Eina does this: >>>>>> >>>>>> #include <Eina.h> >>>>>> #include <pthread.h> >>>>>> >>>>>> The user will not have the possibility to features available with >>>>>> _GNU_SOURCE (like CPU_SET for example. I have that problem with Enesim), >>>>>> except by defining it just before including Eina.h. Which is not the best >>>>>> solution, I think. >>>>>> >>>>>> The problem, here, is that lock stuff is only inlined functions. The >>>>>> problem will be solved if they are in a source file. Maybe at the >>>>>> beginning, having these functions inlined was interesting because they >>>>>> were short. I'm not sure that keeping them inlined is really useful, now. >>>>> >>>>> As from a performance point of view, it really matter to have them >>>>> inlined or not. Function call does cost. >>>>> >>>>>> Another solution would be to define _GNU_SOURCE before including >>>>>> pthread.h (maybe under some conditions). But is it a good solution too ? >>>>>> >>>>>> Honestly, I don't know what the best solution is. So if someone knows how >>>>>> to properly fix that problem... >>>>> >>>>> I have always started to put libc header first if I need them directly >>>>> and then include other library. This just solve this kind of issue. So >>>>> I don't thing it's an issue to solve. >>>> >>>> well, if you think that everyone on earth must code like you... >>> >>> Actually, what would you say to someone that put #ifdef HAVE_CONFIG_H >>> at the end of the C file. That's bad idea and that's the same here. >>> It's just sane to put config first, >> >> hell, just look at raster's use of headers : he puts everything in a >> _private.h (config.h too). If he wants to use Eina: >> >> #include Eina.h >> >> #include "exported_header.h" >> #include "***_private.h" >> >> and boum, it will fail with Eina if he wants to use CPU_SET. He will have >> to include config.h before Eina.h in a specific source file, while it's >> alreay included in his _private.h. > > it's like that because no efl headers ever BEFORE relied on the "hosts" > config.h - in fact they NEVER EVER EVER should do thbis. the header from a lib > should provide the same features always -r egardless of what is in the apps > config.h. sure - the app CAN try and disable features by playing with #define > and #undef games but it should not accidentally happen - ie all #ifdefs should > be namespaced OR be compiler or architecture or platform specific. relying in > #define __GNU_SOURCE or other friends to be defined by the app is bad. > >> Don't tell me how to use these headers. In case you don't remember, it's >> ME who moved all the inclusion of the headers from the *_private.h to the >> source file, to avoid such problems (especially on Windows where it's even >> more evil). > > that's a REAL pain in the arse as then the top of every file has the same set > of 2, 5, 10, 20 header includes. it's maintenance hell. 1) It's a matter of taste : I find cleaner to include only the needed header files. 2) it's not a maintainance hell : I did it once, for several libs. As they are stable, *no* maintainance is needed. So i don't see why it is a pain, nor for you (who do not touch them anyway), or for me (who maintain anyway those includes). So everyone is happy. Can you really think that I find that bothering, me, the current autotools and Windows port maintainer ? :-) 3) with that method, one of the problem of Eina i raised (I insist :-) ) would have been solved without including config.h before your *_private.h of each source file using features allowed by, e.g. _GNU_SOURCE (like CPU_SET) Vincent > i put them all into the > same common/private header and even if not all are used in each file, it > provides a global scope for the app/lib/project that you don't have to keep > re-figuring-out per file. you use eet* in file a then later need it in file b > but find Eet.h isn't included so u have to go fix it again when it can just be a > common include. |
From: Carsten H. (T. R. <ra...@ra...> - 2011-11-01 02:03:47
|
On Tue, 1 Nov 2011 01:23:01 +0100 (CET) Vincent Torri <vt...@un...> said: > On Tue, 1 Nov 2011, Carsten Haitzler (The Rasterman) wrote: > > > On Wed, 26 Oct 2011 12:37:12 +0200 (CEST) Vincent Torri > > <vt...@un...> said: > > > >> > >> > >> On Wed, 26 Oct 2011, Cedric BAIL wrote: > >> > >>> On Wed, Oct 26, 2011 at 11:10 AM, Vincent Torri <vt...@un...> > >>> wrote: > >>>> On Wed, 26 Oct 2011, Cedric BAIL wrote: > >>>>> On Wed, Oct 26, 2011 at 10:55 AM, Vincent Torri <vt...@un...> > >>>>> wrote: > >>>>>> Eina includes eina_inline_lock_posix.h on something else than Windows, > >>>>>> hence pthread.h. _GNU_SOURCE is not defined. > >>>>>> > >>>>>> Suppose now that a user of Eina does this: > >>>>>> > >>>>>> #include <Eina.h> > >>>>>> #include <pthread.h> > >>>>>> > >>>>>> The user will not have the possibility to features available with > >>>>>> _GNU_SOURCE (like CPU_SET for example. I have that problem with > >>>>>> Enesim), except by defining it just before including Eina.h. Which is > >>>>>> not the best solution, I think. > >>>>>> > >>>>>> The problem, here, is that lock stuff is only inlined functions. The > >>>>>> problem will be solved if they are in a source file. Maybe at the > >>>>>> beginning, having these functions inlined was interesting because they > >>>>>> were short. I'm not sure that keeping them inlined is really useful, > >>>>>> now. > >>>>> > >>>>> As from a performance point of view, it really matter to have them > >>>>> inlined or not. Function call does cost. > >>>>> > >>>>>> Another solution would be to define _GNU_SOURCE before including > >>>>>> pthread.h (maybe under some conditions). But is it a good solution > >>>>>> too ? > >>>>>> > >>>>>> Honestly, I don't know what the best solution is. So if someone knows > >>>>>> how to properly fix that problem... > >>>>> > >>>>> I have always started to put libc header first if I need them directly > >>>>> and then include other library. This just solve this kind of issue. So > >>>>> I don't thing it's an issue to solve. > >>>> > >>>> well, if you think that everyone on earth must code like you... > >>> > >>> Actually, what would you say to someone that put #ifdef HAVE_CONFIG_H > >>> at the end of the C file. That's bad idea and that's the same here. > >>> It's just sane to put config first, > >> > >> hell, just look at raster's use of headers : he puts everything in a > >> _private.h (config.h too). If he wants to use Eina: > >> > >> #include Eina.h > >> > >> #include "exported_header.h" > >> #include "***_private.h" > >> > >> and boum, it will fail with Eina if he wants to use CPU_SET. He will have > >> to include config.h before Eina.h in a specific source file, while it's > >> alreay included in his _private.h. > > > > it's like that because no efl headers ever BEFORE relied on the "hosts" > > config.h - in fact they NEVER EVER EVER should do thbis. the header from a > > lib should provide the same features always -r egardless of what is in the > > apps config.h. sure - the app CAN try and disable features by playing with > > #define and #undef games but it should not accidentally happen - ie all > > ##ifdefs should > > be namespaced OR be compiler or architecture or platform specific. relying > > in > > #define __GNU_SOURCE or other friends to be defined by the app is bad. > > > >> Don't tell me how to use these headers. In case you don't remember, it's > >> ME who moved all the inclusion of the headers from the *_private.h to the > >> source file, to avoid such problems (especially on Windows where it's even > >> more evil). > > > > that's a REAL pain in the arse as then the top of every file has the same > > set of 2, 5, 10, 20 header includes. it's maintenance hell. > > 1) It's a matter of taste : I find cleaner to include only the needed > header files. true. i prefer my c files with less stuff at the top. :) > 2) it's not a maintainance hell : I did it once, for several libs. As they > are stable, *no* maintainance is needed. So i don't see why it is a pain, > nor for you (who do not touch them anyway), or for me (who maintain anyway > those includes). So everyone is happy. Can you really think that I find > that bothering, me, the current autotools and Windows port maintainer ? > :-) well it's painful during development... i've had to add headers a few times before because of this as i expected to be able to use the same apis in one part of a lib as another. > 3) with that method, one of the problem of Eina i raised (I insist :-) ) > would have been solved without including config.h before your *_private.h > of each source file using features allowed by, e.g. _GNU_SOURCE (like > CPU_SET) yeah - it would solve it, but its a bi-product - an inadvertent "fix" that was not intended. if Eina.h RELIES on the app including config.h and defining __GNU_SOURCE to work then that's wrong. if anything it probably should do something like: #ifndef __GNU_SOURCE #define NO_GNU_SOURCE #define __GNU_SOURCE 1 ...(rest of eina header here) #ifdef NO_GNU_SOURCE #undef NO_GNU_SOURCE #undef __GNU_SOURCE #endif at least then the eina features would be isolated from the app itself (if eina really needs/wants __GNU_SOURCE set) > Vincent > > > i put them all into the > > same common/private header and even if not all are used in each file, it > > provides a global scope for the app/lib/project that you don't have to keep > > re-figuring-out per file. you use eet* in file a then later need it in file > > b but find Eet.h isn't included so u have to go fix it again when it can > > just be a common include. > -- ------------- Codito, ergo sum - "I code, therefore I am" -------------- The Rasterman (Carsten Haitzler) ra...@ra... |
From: Carsten H. (T. R. <ra...@ra...> - 2011-11-01 01:44:56
|
On Wed, 26 Oct 2011 10:55:58 +0200 (CEST) Vincent Torri <vt...@un...> said: > > Hey > > Eina includes eina_inline_lock_posix.h on something else than Windows, > hence pthread.h. _GNU_SOURCE is not defined. > > Suppose now that a user of Eina does this: > > #include <Eina.h> > #include <pthread.h> > > The user will not have the possibility to features available with > _GNU_SOURCE (like CPU_SET for example. I have that problem with Enesim), > except by defining it just before including Eina.h. Which is not the best > solution, I think. > > The problem, here, is that lock stuff is only inlined functions. The > problem will be solved if they are in a source file. Maybe at the > beginning, having these functions inlined was interesting because they > were short. I'm not sure that keeping them inlined is really useful, now. > > Another solution would be to define _GNU_SOURCE before including pthread.h > (maybe under some conditions). But is it a good solution too ? > > Honestly, I don't know what the best solution is. So if someone knows how > to properly fix that problem... > > Vincent i agree with you - putting actual functions inside headers as macros or inlines has big problems when those functions: 1. can be big (that means maybe more than 10 lines of code) - this causes code bloat as the function is duplicated in the app every time its used 2. if the function must serve as a compatibility system - eg change behavior based on compiletime setup and maybe change from install to install of the library (feature is there or is not). once app is compiled, then what the function does can never change regardless of whatever we change in eina, so doing macors and inlines is best limited to cases where the content will NEVER change AND where performance is so absolutely necessary that we must make them inlined. -- ------------- Codito, ergo sum - "I code, therefore I am" -------------- The Rasterman (Carsten Haitzler) ra...@ra... |