On 09/12/15 15:09, Amitesh Singh wrote: > 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.
That's a good reason (though again, in this time it's not correct). In the future, please mention your benchmarks in the commit, and don't call it a fix, but a performance enhancement, or whatever. Thanks, Tom. > > 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 > ------------------------------------------------------------------------------ _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
