[Spice-devel,03/12] qxl-wddm-dod: Introduce TimeMeasurement class for timing debugging

Submitted by Yuri Benditovich on March 12, 2017, 8:45 a.m.

Details

Message ID 1489308309-9728-4-git-send-email-yuri.benditovich@daynix.com
State New
Headers show
Series "Set of patches for further support of VSync" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Yuri Benditovich March 12, 2017, 8:45 a.m.
In release build this class resolved to empty statements.
In debug build it is useful for measurement of execution time.

Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
 qxldod/QxlDod.h | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

Patch hide | download patch | download mbox

diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
index f441f4b..324b940 100755
--- a/qxldod/QxlDod.h
+++ b/qxldod/QxlDod.h
@@ -225,6 +225,41 @@  typedef struct _CURRENT_BDD_MODE
     } FrameBuffer;
 } CURRENT_BDD_MODE;
 
+#if DBG
+class TimeMeasurement
+{
+public:
+    TimeMeasurement()
+    {
+        Start();
+    }
+    void Start()
+    {
+        KeQuerySystemTime(&li1);
+    }
+    void Stop()
+    {
+        KeQuerySystemTime(&li2);
+    }
+    ULONG Diff()
+    {
+        return (ULONG)((li2.QuadPart - li1.QuadPart) / 10000);
+    }
+protected:
+    LARGE_INTEGER li1;
+    LARGE_INTEGER li2;
+};
+#else
+class TimeMeasurement
+{
+public:
+    TimeMeasurement() {}
+    void Start() {}
+    void Stop() {}
+    ULONG Diff() { return 0; }
+};
+#endif
+
 class QxlDod;
 
 class HwDeviceInterface {

Comments

> 
> In release build this class resolved to empty statements.
> In debug build it is useful for measurement of execution time.
> 
> Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> ---
>  qxldod/QxlDod.h | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
> index f441f4b..324b940 100755
> --- a/qxldod/QxlDod.h
> +++ b/qxldod/QxlDod.h
> @@ -225,6 +225,41 @@ typedef struct _CURRENT_BDD_MODE
>      } FrameBuffer;
>  } CURRENT_BDD_MODE;
>  
> +#if DBG
> +class TimeMeasurement
> +{
> +public:
> +    TimeMeasurement()
> +    {
> +        Start();
> +    }
> +    void Start()
> +    {
> +        KeQuerySystemTime(&li1);
> +    }
> +    void Stop()
> +    {
> +        KeQuerySystemTime(&li2);
> +    }
> +    ULONG Diff()
> +    {
> +        return (ULONG)((li2.QuadPart - li1.QuadPart) / 10000);
> +    }
> +protected:

This seems to indicate that the class is written to be a base
class but there's no virtual destructor.

> +    LARGE_INTEGER li1;
> +    LARGE_INTEGER li2;
> +};
> +#else
> +class TimeMeasurement
> +{
> +public:
> +    TimeMeasurement() {}
> +    void Start() {}
> +    void Stop() {}
> +    ULONG Diff() { return 0; }
> +};
> +#endif
> +

It's also possible to use the class in weird way having negative numbers
(just call Start after Stop). Would not something like this prevent these issues?

class TimeMeasurement
{
public:
    TimeMeasurement()
    {
#if DBG
        KeQuerySystemTime(&li_start);
#endif
    }
    ULONG Elapsed() const 
    {
#if DBG
        LARGE_INTEGER li_end;
        KeQuerySystemTime(&li_end);
        return (ULONG)((li_end.QuadPart - li_end.QuadPart) / 10000);
#else
        return 0; 
#endif
    }
private:
#if DBG
    LARGE_INTEGER li1;
#endif
};


>  class QxlDod;
>  
>  class HwDeviceInterface {

Frediano
On Tue, Mar 21, 2017 at 2:04 PM, Frediano Ziglio <fziglio@redhat.com> wrote:

> >
> > In release build this class resolved to empty statements.
> > In debug build it is useful for measurement of execution time.
> >
> > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> > ---
> >  qxldod/QxlDod.h | 35 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> >
> > diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
> > index f441f4b..324b940 100755
> > --- a/qxldod/QxlDod.h
> > +++ b/qxldod/QxlDod.h
> > @@ -225,6 +225,41 @@ typedef struct _CURRENT_BDD_MODE
> >      } FrameBuffer;
> >  } CURRENT_BDD_MODE;
> >
> > +#if DBG
> > +class TimeMeasurement
> > +{
> > +public:
> > +    TimeMeasurement()
> > +    {
> > +        Start();
> > +    }
> > +    void Start()
> > +    {
> > +        KeQuerySystemTime(&li1);
> > +    }
> > +    void Stop()
> > +    {
> > +        KeQuerySystemTime(&li2);
> > +    }
> > +    ULONG Diff()
> > +    {
> > +        return (ULONG)((li2.QuadPart - li1.QuadPart) / 10000);
> > +    }
> > +protected:
>
> This seems to indicate that the class is written to be a base
> class but there's no virtual destructor.
>

This just indicated that these members are not intended to be accessed
directly.
No derived classes in plans.


>
> > +    LARGE_INTEGER li1;
> > +    LARGE_INTEGER li2;
> > +};
> > +#else
> > +class TimeMeasurement
> > +{
> > +public:
> > +    TimeMeasurement() {}
> > +    void Start() {}
> > +    void Stop() {}
> > +    ULONG Diff() { return 0; }
> > +};
> > +#endif
> > +
>
> It's also possible to use the class in weird way having negative numbers
> (just call Start after Stop). Would not something like this prevent these
> issues?
>
> class TimeMeasurement
> {
> public:
>     TimeMeasurement()
>     {
> #if DBG
>         KeQuerySystemTime(&li_start);
> #endif
>     }
>     ULONG Elapsed() const
>     {
> #if DBG
>         LARGE_INTEGER li_end;
>         KeQuerySystemTime(&li_end);
>         return (ULONG)((li_end.QuadPart - li_end.QuadPart) / 10000);
> #else
>         return 0;
> #endif
>     }
> private:
> #if DBG
>     LARGE_INTEGER li1;
> #endif
> };
>
> This way is much less easy when used during debugging/profiling
in different procedures. Declaring several timers at single line of the
code does not allow
to start and stop each one of them when needed. During profiling different
timers are
started/stopped independently and printed together.

>
> >  class QxlDod;
> >
> >  class HwDeviceInterface {
>
> Frediano
>