[E-devel] [EGIT] [core/enlightenment] master 05/06: reduce include deps for enlightenment

Discussion:

[E-devel] [EGIT] [core/enlightenment] master 05/06: reduce include deps for enlightenment_imc binary

Carsten Haitzler (The Rasterman)

2017-07-18 00:11:37 UTC

On Mon, 17 Jul 2017 05:14:35 -0700 Mike Blumenkrantz
<***@gmail.com> said:

what is the actual value of doing this? you go from a 1 line include to many
lines that can easily break (as already happened a few days back). change
e_config_data.h content to depend on something e.h defines and this breaks
again.

why do this? it's a lot more copy & pasting for sure adding the #define
E_TYPEDEFS and #undef for it - include .h twice for defining types vs the
rest...

what is the value of this? i do see downsides. the only upside i see is
"slightly faster build time". enlightenment_imc is not like it's a binary that
runs often or all the time. it's just a tool like eet or edje_cc etc.

discomfitor pushed a commit to branch master.
http://git.enlightenment.org/core/enlightenment.git/commit/?id=f8de1a3f887f9693a0ff680de62971e0bcfcc0e3
commit f8de1a3f887f9693a0ff680de62971e0bcfcc0e3
Date: Mon Jul 17 08:14:28 2017 -0400
reduce include deps for enlightenment_imc binary
---
src/bin/e_config_data.c | 8 +++++++-
src/bin/e_imc_main.c | 14 ++++++++++++--
src/bin/e_intl_data.c | 17 +++++++++++++++--
3 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/src/bin/e_config_data.c b/src/bin/e_config_data.c
index 8dd90339f..18b13689e 100644
--- a/src/bin/e_config_data.c
+++ b/src/bin/e_config_data.c
@@ -1,4 +1,10 @@
-#include "e.h"
+#include <Eina.h>
+#include <Eet.h>
+#include "e_macros.h"
+#define E_TYPEDEFS
+#include "e_config_data.h"
+#undef E_TYPEDEFS
+#include "e_config_data.h"
static Eina_Hash *config_hash = NULL;
diff --git a/src/bin/e_imc_main.c b/src/bin/e_imc_main.c
index 8c5adc296..cef9253ad 100644
--- a/src/bin/e_imc_main.c
+++ b/src/bin/e_imc_main.c
@@ -1,4 +1,14 @@
-#include "e.h"
+#include <Eina.h>
+#include <Eet.h>
+#include <Ecore_File.h>
+#include "e_macros.h"
+#define E_TYPEDEFS
+#include "e_config_data.h"
+#include "e_intl_data.h"
+#undef E_TYPEDEFS
+#include "e_config_data.h"
+#include "e_intl_data.h"
+
static void _e_help(void);
@@ -181,7 +191,7 @@ main(int argc, char **argv)
}
e_intl_input_method_config_free(read_imc);
- E_FREE(write_imc);
+ free(write_imc);
eet_close(ef);
e_intl_data_shutdown();
eet_shutdown();
diff --git a/src/bin/e_intl_data.c b/src/bin/e_intl_data.c
index bd02c8110..5580a3e22 100644
--- a/src/bin/e_intl_data.c
+++ b/src/bin/e_intl_data.c
@@ -1,4 +1,17 @@
-#include "e.h"
+#include <Eina.h>
+#include <Eet.h>
+#define E_TYPEDEFS
+#include "e_config_data.h"
+#include "e_intl_data.h"
+#undef E_TYPEDEFS
+#ifndef E_API
+# define E_API
+#endif
+#ifndef EINTERN
+# define EINTERN
+#endif
+#include "e_config_data.h"
+#include "e_intl_data.h"
/* This file is the counterpart for data storage of e_intl */
/* This only needs to be separate because the e_imc binary and other third
(E_Input_Method_Config *imc) if (imc->ecore_imf_module) eina_stringshare_del
(imc->ecore_imf_module); if (imc->e_im_exec) eina_stringshare_del
(imc->e_im_exec); if (imc->e_im_setup_exec) eina_stringshare_del
(imc->e_im_setup_exec);
- E_FREE(imc);
+ free(imc);
}
}
--

--
------------- Codito, ergo sum - "I code, therefore I am" --------------
The Rasterman (Carsten Haitzler) ***@rasterman.com

Mike Blumenkrantz

2017-07-18 22:15:32 UTC

Permalink

Various targets were overlinking/overincluding with autotools which was
making non-autotools builds more difficult.

Post by Carsten Haitzler (The Rasterman)
On Mon, 17 Jul 2017 05:14:35 -0700 Mike Blumenkrantz
what is the actual value of doing this? you go from a 1 line include to many
lines that can easily break (as already happened a few days back). change
e_config_data.h content to depend on something e.h defines and this breaks
again.
why do this? it's a lot more copy & pasting for sure adding the #define
E_TYPEDEFS and #undef for it - include .h twice for defining types vs the
rest...
what is the value of this? i do see downsides. the only upside i see is
"slightly faster build time". enlightenment_imc is not like it's a binary that
runs often or all the time. it's just a tool like eet or edje_cc etc.

discomfitor pushed a commit to branch master.

http://git.enlightenment.org/core/enlightenment.git/commit/?id=f8de1a3f887f9693a0ff680de62971e0bcfcc0e3

commit f8de1a3f887f9693a0ff680de62971e0bcfcc0e3
Date: Mon Jul 17 08:14:28 2017 -0400
reduce include deps for enlightenment_imc binary
---
src/bin/e_config_data.c | 8 +++++++-
src/bin/e_imc_main.c | 14 ++++++++++++--
src/bin/e_intl_data.c | 17 +++++++++++++++--
3 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/src/bin/e_config_data.c b/src/bin/e_config_data.c
index 8dd90339f..18b13689e 100644
--- a/src/bin/e_config_data.c
+++ b/src/bin/e_config_data.c
@@ -1,4 +1,10 @@
-#include "e.h"
+#include <Eina.h>
+#include <Eet.h>
+#include "e_macros.h"
+#define E_TYPEDEFS
+#include "e_config_data.h"
+#undef E_TYPEDEFS
+#include "e_config_data.h"
static Eina_Hash *config_hash = NULL;
diff --git a/src/bin/e_imc_main.c b/src/bin/e_imc_main.c
index 8c5adc296..cef9253ad 100644
--- a/src/bin/e_imc_main.c
+++ b/src/bin/e_imc_main.c
@@ -1,4 +1,14 @@
-#include "e.h"
+#include <Eina.h>
+#include <Eet.h>
+#include <Ecore_File.h>
+#include "e_macros.h"
+#define E_TYPEDEFS
+#include "e_config_data.h"
+#include "e_intl_data.h"
+#undef E_TYPEDEFS
+#include "e_config_data.h"
+#include "e_intl_data.h"
+
static void _e_help(void);
@@ -181,7 +191,7 @@ main(int argc, char **argv)
}
e_intl_input_method_config_free(read_imc);
- E_FREE(write_imc);
+ free(write_imc);
eet_close(ef);
e_intl_data_shutdown();
eet_shutdown();
diff --git a/src/bin/e_intl_data.c b/src/bin/e_intl_data.c
index bd02c8110..5580a3e22 100644
--- a/src/bin/e_intl_data.c
+++ b/src/bin/e_intl_data.c
@@ -1,4 +1,17 @@
-#include "e.h"
+#include <Eina.h>
+#include <Eet.h>
+#define E_TYPEDEFS
+#include "e_config_data.h"
+#include "e_intl_data.h"
+#undef E_TYPEDEFS
+#ifndef E_API
+# define E_API
+#endif
+#ifndef EINTERN
+# define EINTERN
+#endif
+#include "e_config_data.h"
+#include "e_intl_data.h"
/* This file is the counterpart for data storage of e_intl */
/* This only needs to be separate because the e_imc binary and other

third

(E_Input_Method_Config *imc) if (imc->ecore_imf_module)

eina_stringshare_del

(imc->ecore_imf_module); if (imc->e_im_exec) eina_stringshare_del
(imc->e_im_exec); if (imc->e_im_setup_exec) eina_stringshare_del
(imc->e_im_setup_exec);
- E_FREE(imc);
+ free(imc);
}
}
--

--
------------- Codito, ergo sum - "I code, therefore I am" --------------

Carsten Haitzler (The Rasterman)

2017-07-18 23:43:50 UTC

Permalink

On Tue, 18 Jul 2017 22:15:32 +0000 Mike Blumenkrantz

Post by Mike Blumenkrantz
Various targets were overlinking/overincluding with autotools which was
making non-autotools builds more difficult.

what non-autotools builds? e has only one build system. that's what i'm staring
at...

and is it really more difficult? if you have the same identical linking/cflags
etc. shouldn't it actually be simpler? if you try and specialize with different
link/include flags be more work if there were such a mythical "other build
system" ?

Post by Mike Blumenkrantz

discomfitor pushed a commit to branch master.

http://git.enlightenment.org/core/enlightenment.git/commit/?id=f8de1a3f887f9693a0ff680de62971e0bcfcc0e3

third

(E_Input_Method_Config *imc) if (imc->ecore_imf_module)

eina_stringshare_del

(imc->ecore_imf_module); if (imc->e_im_exec) eina_stringshare_del
(imc->e_im_exec); if (imc->e_im_setup_exec) eina_stringshare_del
(imc->e_im_setup_exec);
- E_FREE(imc);
+ free(imc);
}
}
--

--
------------- Codito, ergo sum - "I code, therefore I am" --------------

--
------------- Codito, ergo sum - "I code, therefore I am" --------------
The Rasterman (Carsten Haitzler) ***@rasterman.com

Carsten Haitzler (The Rasterman)

2017-07-19 05:44:14 UTC

Permalink

On Wed, 19 Jul 2017 08:43:50 +0900 Carsten Haitzler (The Rasterman)

Post by Carsten Haitzler (The Rasterman)
On Tue, 18 Jul 2017 22:15:32 +0000 Mike Blumenkrantz

Post by Mike Blumenkrantz
Various targets were overlinking/overincluding with autotools which was
making non-autotools builds more difficult.

what non-autotools builds? e has only one build system. that's what i'm
staring at...
and is it really more difficult? if you have the same identical linking/cflags
etc. shouldn't it actually be simpler? if you try and specialize with
different link/include flags be more work if there were such a mythical
"other build system" ?

ok. i checked your meson.buld. it'd be EASIER to not have it be different. just
use deps_e for enlightenment_imc etc. too. same deps_e. it's less code in the
meson.build to do it this way. yes it's over-linking but that'd only lead to:

1. slower compiles
2. slower start times for the binaries etc. affected.

for enightenment_imc this is really a "whatever".

Post by Carsten Haitzler (The Rasterman)

Post by Mike Blumenkrantz

discomfitor pushed a commit to branch master.

http://git.enlightenment.org/core/enlightenment.git/commit/?id=f8de1a3f887f9693a0ff680de62971e0bcfcc0e3

third

(E_Input_Method_Config *imc) if (imc->ecore_imf_module)

eina_stringshare_del

(imc->ecore_imf_module); if (imc->e_im_exec) eina_stringshare_del
(imc->e_im_exec); if (imc->e_im_setup_exec) eina_stringshare_del
(imc->e_im_setup_exec);
- E_FREE(imc);
+ free(imc);
}
}
--

--
------------- Codito, ergo sum - "I code, therefore I am" --------------

--
------------- Codito, ergo sum - "I code, therefore I am" --------------
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
enlightenment-devel mailing list
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

--
------------- Codito, ergo sum - "I code, therefore I am" --------------
The Rasterman (Carsten Haitzler) ***@rasterman.com

Carsten Haitzler (The Rasterman)

2017-07-19 06:06:04 UTC

Permalink

On Wed, 19 Jul 2017 14:44:14 +0900 Carsten Haitzler (The Rasterman)

Post by Carsten Haitzler (The Rasterman)
On Wed, 19 Jul 2017 08:43:50 +0900 Carsten Haitzler (The Rasterman)

Post by Carsten Haitzler (The Rasterman)
On Tue, 18 Jul 2017 22:15:32 +0000 Mike Blumenkrantz

Post by Mike Blumenkrantz
Various targets were overlinking/overincluding with autotools which was
making non-autotools builds more difficult.

what non-autotools builds? e has only one build system. that's what i'm
staring at...
and is it really more difficult? if you have the same identical
linking/cflags etc. shouldn't it actually be simpler? if you try and
specialize with different link/include flags be more work if there were
such a mythical "other build system" ?

ok. i checked your meson.buld. it'd be EASIER to not have it be different.
just use deps_e for enlightenment_imc etc. too. same deps_e. it's less code
in the meson.build to do it this way. yes it's over-linking but that'd only
1. slower compiles
2. slower start times for the binaries etc. affected.
for enightenment_imc this is really a "whatever".

even better. it seems either gcc or ninja use options that auto-optimize the
linking to ONLY link when symbols are used from that library... so even if you
use deps_e that links to everything - elementary included... it will not
actually link ... it strips out unused libs. :) so... the source is simpler.
1 include lines instead of 10 in e_imc_main.c - also doesn't need the same 1
-> 10 lines changes in the other src files too making the code longer and more
fragile... it ALSO the meson.build file is simpler.

dependencies: [dep_eina, dep_eet, dep_ecore_file]

becomes

dependencies: deps_e

AND the linker/compiler even strip out what isn't needed in the end so actually
produce an optimal binary. so... the ONLY downside is maybe slightly slower
compile time at linking. this wasn't an issue you were pointing out... and i
just was timing the differences for enlightenment_imc with build of that c file
+ link...

in fact using deps_e takes 0.25 sec or so (0.254 0.246 0.222 over 3 runs)
using the custom deps list takes 0.32 sec or so (0.318 0.340 0.323 over 3 runs)

so it is actually measurably SLOWER to use a custom dependencies list
(interesting... i didn't expect that). it's a longer/larger meson build file
AND it is significantly more code in source that is more fragile (as was the
reason i reverted).

my point is... don't so this. it's just bad. in every possible way. more
fragile code needing more maintenance that more easily breaks that is longer.
it takes longer to compile and it doesn't have any more libraries linked due to
linkers/build sys seemingly optimizing that... :)

Post by Carsten Haitzler (The Rasterman)

Post by Mike Blumenkrantz

Post by Carsten Haitzler (The Rasterman)
On Mon, 17 Jul 2017 05:14:35 -0700 Mike Blumenkrantz
what is the actual value of doing this? you go from a 1 line include to many
lines that can easily break (as already happened a few days back).
change e_config_data.h content to depend on something e.h defines and
this breaks again.
why do this? it's a lot more copy & pasting for sure adding the #define
E_TYPEDEFS and #undef for it - include .h twice for defining types vs
the rest...
what is the value of this? i do see downsides. the only upside i see is
"slightly faster build time". enlightenment_imc is not like it's a binary that
runs often or all the time. it's just a tool like eet or edje_cc etc.

discomfitor pushed a commit to branch master.

http://git.enlightenment.org/core/enlightenment.git/commit/?id=f8de1a3f887f9693a0ff680de62971e0bcfcc0e3

third

(E_Input_Method_Config *imc) if (imc->ecore_imf_module)

eina_stringshare_del

(imc->ecore_imf_module); if (imc->e_im_exec) eina_stringshare_del
(imc->e_im_exec); if (imc->e_im_setup_exec) eina_stringshare_del
(imc->e_im_setup_exec);
- E_FREE(imc);
+ free(imc);
}
}
--

--
------------- Codito, ergo sum - "I code, therefore I am" --------------

--
------------- Codito, ergo sum - "I code, therefore I am" --------------
The Rasterman (Carsten Haitzler) ***@rasterman.com