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