On Thu, Mar 03, 2016 at 09:50:26AM -0500, Frediano Ziglio wrote:
> > diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
> > index aae47c7..3dfbdfb 100644
> > --- a/server/gstreamer-encoder.c
> > +++ b/server/gstreamer-encoder.c
> > @@ -188,14 +188,70 @@ static void set_appsrc_caps(SpiceGstEncoder *encoder)
> > g_object_set(G_OBJECT(encoder->appsrc), "caps", encoder->src_caps,
> > NULL);
> > }
> >
> > +static int physical_core_count = 0;
> > +static int get_physical_core_count(void)
> > +{
> > + if (!physical_core_count) {
> > +#ifdef HAVE_G_GET_NUMPROCESSORS
> > + physical_core_count = g_get_num_processors();
> > +#endif
>
> you could use sysconf if g_get_num_processors is not detected.
>
> > + if (system("egrep -l '^flags\\b.*: .*\\bht\\b' /proc/cpuinfo
> > >/dev/null 2>&1") == 0) {
> > + /* Hyperthreading is enabled so divide by two to get the number
> > + * of physical cores.
> > + */
> > + physical_core_count = physical_core_count / 2;
> > + }
>
> Or as you are using /proc/cpuinfo detect from it.
>
> I'm not sure we should consider compatibility with other processors (like
> ARM).In general, I don't think we want to silently use as many cores as possible for cpu intensive work. This will break down as soon as 2 VMs trigger the streaming code at the same time, they both will be trying to use all the available physical cores. Even with a single VM, it could use 4 vcpus and do CPU intensive tasks on some of them, thus the vp8 encoder would fight for these resources. I understand in some controlled setups you may want to give more threads to the encoder, but I think doing it automatically is going to cause us some issues. I'm not sure what is the best way of making this configurable is though :( Can you split this patch for now so that the rest of the vp8 support can go in? Christophe
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/spice-devel
