Re: gdi32: Optionally return the type from GDI_GetObjPtr().

[Dmitry Timoshkov]

Huw Davies  wrote:
> This enables get_dc_obj() to check the type without calling GetObjectType()
> and thus it saves additional calls to Enter/LeaveCriticalSection().
> Signed-off-by: Huw Davies
>
> This reduces the runtime of the dib tests by about 8% and achieves a similar
> increase in performance with the drawing phase of a (gdi heavy) app I'm
> looking at.

...
> - bmp = GDI_GetObjPtr( hbitmap, OBJ_BITMAP );
> - bmp = GDI_GetObjPtr( hbitmap, OBJ_BITMAP, NULL );

Did you consider introducing GDI_GetObjPtrEx() with additional pointer to
type, and making a shortcut by defining/calling it by a not Ex version?
That would simplify the patch and avoid penalizing/changing the callers
that don't need the type.

gdi32: Optionally return the type from GDI_GetObjPtr().

[Huw Davies]

On Fri, Jul 08, 2016 at 05:24:29PM +0800, Dmitry Timoshkov wrote:
> Huw Davies  wrote:
> > This enables get_dc_obj() to check the type without calling GetObjectType()
> > and thus it saves additional calls to Enter/LeaveCriticalSection().
> > Signed-off-by: Huw Davies
> >
> > This reduces the runtime of the dib tests by about 8% and achieves a similar
> > increase in performance with the drawing phase of a (gdi heavy) app I'm
> > looking at.
>
> ...
> > - bmp = GDI_GetObjPtr( hbitmap, OBJ_BITMAP );
> > - bmp = GDI_GetObjPtr( hbitmap, OBJ_BITMAP, NULL );
>
> Did you consider introducing GDI_GetObjPtrEx() with additional pointer to
> type, and making a shortcut by defining/calling it by a not Ex version?

No.
> That would simplify the patch and avoid penalizing/changing the callers
> that don't need the type.

We don't don't need to retain backwards compatibility so I don't see
the point, indeed an additional 'Ex' version would be more
confusing IMO.  Almost all of the time in that function is spent
grabbing the crit sec, so any time saved by not filling in the out
param is totally negligible.

Huw.

gdi32: Optionally return the type from GDI_GetObjPtr().

[Alexandre Julliard]

Huw Davies  writes:
> On Fri, Jul 08, 2016 at 05:24:29PM +0800, Dmitry Timoshkov wrote:
> > Huw Davies  wrote:
> > > This enables get_dc_obj() to check the type without calling GetObjectType()
> > > and thus it saves additional calls to Enter/LeaveCriticalSection().
> > > Signed-off-by: Huw Davies
> > >
> > > This reduces the runtime of the dib tests by about 8% and achieves a similar
> > > increase in performance with the drawing phase of a (gdi heavy) app I'm
> > > looking at.
> >
> > ...
> > > - bmp = GDI_GetObjPtr( hbitmap, OBJ_BITMAP );
> > > - bmp = GDI_GetObjPtr( hbitmap, OBJ_BITMAP, NULL );
> >
> > Did you consider introducing GDI_GetObjPtrEx() with additional pointer to
> > type, and making a shortcut by defining/calling it by a not Ex version?
>
> No.
> > That would simplify the patch and avoid penalizing/changing the callers
> > that don't need the type.
>
> We don't don't need to retain backwards compatibility so I don't see
> the point, indeed an additional 'Ex' version would be more
> confusing IMO.  Almost all of the time in that function is spent
> grabbing the crit sec, so any time saved by not filling in the out
> param is totally negligible.

The performance impact is negligible, but it doesn't make a lot of sense
to return the type when it was already specified by the caller. Some
kind of get_any_obj_ptr(HGDIOBJ, WORD *) function for the any type case
would probably be cleaner.

gdi32: Optionally return the type from GDI_GetObjPtr().

[Huw Davies]

On Fri, Jul 08, 2016 at 08:28:56PM +0900, Alexandre Julliard wrote:
> The performance impact is negligible, but it doesn't make a lot of sense
> to return the type when it was already specified by the caller. Some
> kind of get_any_obj_ptr(HGDIOBJ, WORD *) function for the any type case
> would probably be cleaner.

Hmm, ok.  Yes, that makes sense.  New version sent.

Huw.