On Thu, 10 Dec 2015 22:07:50 +0530 Amitesh Singh <singh.amit...@gmail.com> said:
> On Dec 10, 2015 4:32 AM, "Carsten Haitzler" <ras...@rasterman.com> wrote: > > > > On Wed, 9 Dec 2015 20:39:30 +0530 Amitesh Singh <singh.amit...@gmail.com> > said: > > > > > On Dec 9, 2015 6:20 PM, "Tom Hacohen" <t...@osg.samsung.com> wrote: > > > > > > > > On 09/12/15 12:40, Mike Blumenkrantz wrote: > > > > > I can confirm that Vyacheslav is correct, and even if he did not > mention > > > > > stringshare, this change is still wrong on a fundamental level. > > > > > > > > > > Please revert it. > > > > > > > > What really stands out in this change, is that I don't get the reason > > > > for it. Why was it even done? At worse, it's 0.00000000001% slower > than > > > > after the change, but it's definitely not "more correct". > > > > > > > > Ami, how did you even get to do this fix? as it doesn't, and can't fix > > > > anything. > > > I was profiling tween _ state function (elm label slide ) and this check > > > was getting called every time and it was always false . So I thought to > > > reduce one more check. I think it might not be the case in other > scenarios. > > > > > > But you guys are right.. I would revert it in few mins. > > > > did it speed things up? :) what did your profiling say where most of the > cpu > > time is going? did you try perf and narrow down lines? or callgrind? get > some > > figures per line of code? > Yes. I used callgrind. and what did it say? > > > > as such the check for the ptr first would assume stringshared strings are > there > > and thus a ptr cmp is a fast and quick way of checking before doing a full > > string check. :) both ptrs will be in registers already there regardless > so its > > a cmp + jmp (and on some archs it might use conditional commands after > this > > point too so even avoid the jmp) > > > > > Thanks > > > ami > > > > > > > > -- > > > > Tom. > > > > > > > > > > > > > > On Wed, Dec 9, 2015 at 7:08 AM Vyacheslav Reutskiy < > > > reutskiy....@gmail.com> > > > > > wrote: > > > > > > > > > >> Hello, > > > > >> > > > > >> I'm not sure that this changes is correct. The 'state_name' can be > > > > >> pointer to eina_stringshare and comparing the two pointers faster > > > > >> than strcmp. This fix looks doubtful. > > > > >> > > > > >> -- > > > > >> Viacheslav Reutskiy (rimmed) > > > > >> > > > > >> On Wed, Dec 9, 2015 at 12:20 PM, Amitesh Singh < > amitesh...@samsung.com> > > > > >> wrote: > > > > >> > > > > >>> ami pushed a commit to branch master. > > > > >>> > > > > >>> > > > > >>> > > > > >> > > > > http://git.enlightenment.org/core/efl.git/commit/?id=c892a1cb714fed496cbf5568c4d43880b6fb67b2 > > > > >>> > > > > >>> commit c892a1cb714fed496cbf5568c4d43880b6fb67b2 > > > > >>> Author: Amitesh Singh <amitesh...@samsung.com> > > > > >>> Date: Wed Dec 9 15:46:41 2015 +0530 > > > > >>> > > > > >>> edje: calc - remove pointer comparison while finding part > desc > > > > >>> > > > > >>> Only strcmp comparision is realiable. > > > > >>> @fix > > > > >>> --- > > > > >>> src/lib/edje/edje_calc.c | 3 +-- > > > > >>> 1 file changed, 1 insertion(+), 2 deletions(-) > > > > >>> > > > > >>> diff --git a/src/lib/edje/edje_calc.c b/src/lib/edje/edje_calc.c > > > > >>> index b0742cf..c06e3ac 100644 > > > > >>> --- a/src/lib/edje/edje_calc.c > > > > >>> +++ b/src/lib/edje/edje_calc.c > > > > >>> @@ -460,8 +460,7 @@ _edje_part_description_find(Edje *ed, > > > Edje_Real_Part > > > > >>> *rp, const char *state_name > > > > >>> { > > > > >>> d = ep->other.desc[i]; > > > > >>> > > > > >>> - if (d->state.name && (d->state.name == state_name || > > > > >>> - !strcmp(d->state.name, > state_name))) > > > > >>> + if (d->state.name && (!strcmp(d->state.name, > state_name))) > > > > >>> { > > > > >>> if (!approximate) > > > > >>> { > > > > >>> > > > > >>> -- > > > > >>> > > > > >>> > > > > >>> > > > > >> > > > > >> > > > > ------------------------------------------------------------------------------ > > > > >> _______________________________________________ > > > > >> enlightenment-devel mailing list > > > > >> enlightenment-devel@lists.sourceforge.net > > > > >> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > > > >> > > > > > > > > > ------------------------------------------------------------------------------ > > > > > _______________________________________________ > > > > > enlightenment-devel mailing list > > > > > enlightenment-devel@lists.sourceforge.net > > > > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > > > > > > > > > > > > > > > > > > > > ------------------------------------------------------------------------------ > > > > _______________________________________________ > > > > enlightenment-devel mailing list > > > > enlightenment-devel@lists.sourceforge.net > > > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > > > ------------------------------------------------------------------------------ > > > _______________________________________________ > > > enlightenment-devel mailing list > > > enlightenment-devel@lists.sourceforge.net > > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > > > > > > > > -- > > ------------- Codito, ergo sum - "I code, therefore I am" -------------- > > The Rasterman (Carsten Haitzler) ras...@rasterman.com > > > ------------------------------------------------------------------------------ > _______________________________________________ > enlightenment-devel mailing list > enlightenment-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > -- ------------- Codito, ergo sum - "I code, therefore I am" -------------- The Rasterman (Carsten Haitzler) ras...@rasterman.com ------------------------------------------------------------------------------ _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
