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? 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