Re: [Development] RFC: new moc feature

2016-01-06 Thread Kevin Kofler
Knoll Lars wrote:
> Just as a side note: While perf ensures there’s no collisions between
> valid keys in the hash table, you still end up doing one string comparison
> in the end to ensure that your input string matches the key.

Why? Just document that passing an unknown string is undefined behavior, 
then if your invalid input happens to collide with the hash of something 
valid, it'll just be a synonym, undefined means undefined.

Kevin Kofler

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] RFC: new moc feature

2015-12-07 Thread Matthew Woehlke
On 2015-12-05 07:00, Sean Harmer wrote:
> No idea if all of our
> supported compilers allow hooking in custom preprocessors or not.

Okay, that sentence just scares the pants off me... forget whether or
not the *compilers* support it; what about the build systems? Not
everyone uses qmake. At minimum you'll want/need to support CMake as well.

-- 
Matthew

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] RFC: new moc feature

2015-12-07 Thread Knoll Lars
On 05/12/15 21:37, "Development on behalf of Marc Mutz" 
 wrote:



>On Saturday 05 December 2015 20:20:27 Sean Harmer wrote:
>> I was 
>> just wondering if we could get it down to the theoretical ideal of a 
>> single integer comparison mapped into the finite set of strings in use. 
>> Seems not, without some non-neglible effort.
>
>man 1 gperf ?

Just as a side note: While perf ensures there’s no collisions between valid 
keys in the hash table, you still end up doing one string comparison in the end 
to ensure that your input string matches the key. 

Cheers,
Lars

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] RFC: new moc feature

2015-12-07 Thread Marc Mutz
On Monday 07 December 2015 13:15:13 Knoll Lars wrote:
> On 05/12/15 21:37, "Development on behalf of Marc Mutz"  wrote:
> >On Saturday 05 December 2015 20:20:27 Sean Harmer wrote:
> >> I was
> >> just wondering if we could get it down to the theoretical ideal of a
> >> single integer comparison mapped into the finite set of strings in use.
> >> Seems not, without some non-neglible effort.
> >
> >man 1 gperf ?
> 
> Just as a side note: While perf ensures there’s no collisions between valid
> keys in the hash table, you still end up doing one string comparison in
> the end to ensure that your input string matches the key.

As you always must, because every hash function that maps an infinite-domain 
object such as a string into an object with finite domain (e.g. int) must 
exhibit collisions which need to be handled by comparing the actual objects.

AFAIU, the original idea was to assign an enum value for each unique string (= 
property = property name) automatically, and use the enums instead of the 
strings inside the implementation. In that case, the validation would happen 
at the point of converting the string or whatever is left as a non-ID ID to 
the enum value, and further code could depend on enum value equality alone.

But if you use strings in the implementation, and use the IDs only for quick 
comparision, you cannot rule out that two properties' IDs collide. The 
compiler won't help, since in general the collision may not be from objects at 
the same depth of hierarchy, so the compiler may not see the collision, as the 
equal values then appear in separate switch statements.

But it could be that I misunderstand the situation completely :)

Thanks,
Marc

-- 
Marc Mutz  | Senior Software Engineer
KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
Tel: +49-30-521325470
KDAB - The Qt Experts
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] RFC: new moc feature

2015-12-07 Thread Sean Harmer

On 07/12/2015 15:34, Matthew Woehlke wrote:

On 2015-12-05 07:00, Sean Harmer wrote:

No idea if all of our
supported compilers allow hooking in custom preprocessors or not.

Okay, that sentence just scares the pants off me... forget whether or
not the *compilers* support it; what about the build systems? Not
everyone uses qmake. At minimum you'll want/need to support CMake as well.



Right after investigating a little more, doing it via the build system 
is better. I have too much on right now to actually implement this but 
may get back to it at some point if benchmarks still highlight this type 
of code as being a problem.


Cheers,

Sean

--
Dr Sean Harmer | sean.har...@kdab.com | Managing Director UK
KDAB (UK) Ltd, a KDAB Group company
Tel. +44 (0)1625 809908; Sweden (HQ) +46-563-540090
Mobile: +44 (0)7545 140604
KDAB - Qt Experts

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] RFC: new moc feature

2015-12-05 Thread Marc Mutz
On Saturday 05 December 2015 13:00:44 Sean Harmer wrote:
> So given the above example we'd be able to have something like this 
> (name of macro pending):
> 
>  if (e->type() == NodeUpdated) {
>  const QScenePropertyChangePtr  = 
> qSharedPointerCast(e);
>  switch (propertyChange->propertyNameStringId()) {
>  case qStrId("scale3D"):
>  m_scale = propertyChange->value().value();
>  updateMatrix();
>  break;
>  case qStrId("rotation"):
>  m_rotation = propertyChange->value().value();
>  updateMatrix();
>  break;
>  case qStrId("translation"):
>  m_translation = propertyChange->value().value();
>  updateMatrix();
>  break;
>  case qStrId("enabled"):
>  m_enabled = propertyChange->value().toBool();
>  break;
>   default:
>  qWarning() << "Unknown property update";
>  }
>  }
> 
> Where the qStrId macro expands to nothing and the preprocess tool 
> replaces its contents with the hashed value. No idea if all of our 
> supported compilers allow hooking in custom preprocessors or not.

How do you handle the invariable hash collisions?

What's wrong the the old and trusted

  switch (propName.front()) {
  case 's': if (propName == "scale3D") ...
  case 'r': if (propName == "rotation") ...

(and why the QByteArrayLiteral in the original code)?

Thanks,
Marc

-- 
Marc Mutz  | Senior Software Engineer
KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
Tel: +49-30-521325470
KDAB - The Qt Experts
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] RFC: new moc feature

2015-12-05 Thread Marc Mutz
On Saturday 05 December 2015 20:20:27 Sean Harmer wrote:
> I was 
> just wondering if we could get it down to the theoretical ideal of a 
> single integer comparison mapped into the finite set of strings in use. 
> Seems not, without some non-neglible effort.

man 1 gperf ?

-- 
Marc Mutz  | Senior Software Engineer
KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
Tel: +49-30-521325470
KDAB - The Qt Experts
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] RFC: new moc feature

2015-12-05 Thread Konstantin Tokarev


05.12.2015, 22:29, "Marc Mutz" :
> On Saturday 05 December 2015 20:20:27 Sean Harmer wrote:
>>  I was
>>  just wondering if we could get it down to the theoretical ideal of a
>>  single integer comparison mapped into the finite set of strings in use.
>>  Seems not, without some non-neglible effort.
>
> man 1 gperf ?

IMO, integration of gperf needs too much scaffolding. re2c is more convenient 
for implementing of string switch

-- 
Regards,
Konstantin
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] RFC: new moc feature

2015-12-05 Thread Sean Harmer

Hi Marc,

On 05/12/2015 19:06, Marc Mutz wrote:

On Saturday 05 December 2015 13:00:44 Sean Harmer wrote:

So given the above example we'd be able to have something like this
(name of macro pending):

  if (e->type() == NodeUpdated) {
  const QScenePropertyChangePtr  =
qSharedPointerCast(e);
  switch (propertyChange->propertyNameStringId()) {
  case qStrId("scale3D"):
  m_scale = propertyChange->value().value();
  updateMatrix();
  break;
  case qStrId("rotation"):
  m_rotation = propertyChange->value().value();
  updateMatrix();
  break;
  case qStrId("translation"):
  m_translation = propertyChange->value().value();
  updateMatrix();
  break;
  case qStrId("enabled"):
  m_enabled = propertyChange->value().toBool();
  break;
   default:
  qWarning() << "Unknown property update";
  }
  }

Where the qStrId macro expands to nothing and the preprocess tool
replaces its contents with the hashed value. No idea if all of our
supported compilers allow hooking in custom preprocessors or not.

How do you handle the invariable hash collisions?


Not sure yet. Still thinking this through.

The chances of a collision within the scope of a comparison such as this 
is pretty small (of course dependent upon the quality of the hash 
function). If used within a hash the compiler would complain about 
duplicated case labels.


If the preprocess tool maintains a catalogue of entries in the hash it 
could also warn about collisions to allow the user opportunity to alter 
the names being used.



What's wrong the the old and trusted

   switch (propName.front()) {
   case 's': if (propName == "scale3D") ...
   case 'r': if (propName == "rotation") ...


Indeed, this is something we could adopt now with little effort. I was 
just wondering if we could get it down to the theoretical ideal of a 
single integer comparison mapped into the finite set of strings in use. 
Seems not, without some non-neglible effort.



(and why the QByteArrayLiteral in the original code)?


Simple oversight. Will fix.

Cheers,

Sean

--
Dr Sean Harmer | sean.har...@kdab.com | Managing Director UK
KDAB (UK) Ltd, a KDAB Group company
Tel. +44 (0)1625 809908; Sweden (HQ) +46-563-540090
Mobile: +44 (0)7545 140604
KDAB - Qt Experts

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] RFC: new moc feature

2015-12-05 Thread Marc Mutz
On Saturday 05 December 2015 20:06:22 Marc Mutz wrote:
> invariable

inevitable

-- 
Marc Mutz  | Senior Software Engineer
KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
Tel: +49-30-521325470
KDAB - The Qt Experts
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] RFC: new moc feature

2015-12-05 Thread Sean Harmer

Hi Olivier,

On 05/12/2015 10:22, Olivier Goffart wrote:

On Saturday 5. December 2015 09:30:33 Sean Harmer wrote:

Hi devs,

I'd like to get some feedback on a new feature for moc before we take it
any further than mild musing. The context is Qt3D has some frontend
QObject subclass types, and corresponding backend, non-QObject subclass
types for reasons of allowing us to process data on the backend without
blocking the main thread or locking it.

To get the property values from the frontend objects to the backend we
wrap the data up in QScenePropertyChange and in the backend object we
unpack the data and handle it with code something like this:

  if (e->type() == NodeUpdated) {
  const QScenePropertyChangePtr  =
qSharedPointerCast(e);
  if (propertyChange->propertyName() ==
QByteArrayLiteral("scale3D")) {
  m_scale = propertyChange->value().value();
  updateMatrix();
  } else if (propertyChange->propertyName() ==
QByteArrayLiteral("rotation")) {
  m_rotation = propertyChange->value().value();
  updateMatrix();
  } else if (propertyChange->propertyName() ==
QByteArrayLiteral("translation")) {
  m_translation = propertyChange->value().value();
  updateMatrix();
  } else if (propertyChange->propertyName() ==
QByteArrayLiteral("enabled")) {
  m_enabled = propertyChange->value().toBool();
  }
  }

Not too bad in this case but those cascaded if-else if blocks are not
good when the number of properties is large.

Why not?  Readability or performence?


Both but performance is the main driver as potentially we have a large 
number of string comparisons.





This is where the proposed new feature of moc would come in. If moc were
able to generate an enum where each enum value corresponds to a static
property we would be able to use a switch in the above code.

Is such a feature feasible? Are there reasons why it couldn't work?

I'm afraid this is not feasable. This enum would need to be in the header, and
the moc generated code is not a header.


Ah yes of course. Silly me.


Instead, I would suggest something similar to llvm::StringSwitch
http://code.woboq.org/llvm/llvm/include/llvm/ADT/StringSwitch.h.html


Right, thanks. I've also been considering compile time hashing of the 
strings via constexpr implementation of some hashing algorithm but from 
reading around it seems MSVC2012 won't be up to the job for this approach.


So at present that leaves a build time preprocess tool that goes over 
the sources and replaces marked up strings with their hashed values. 
Plus side is that the work is done at build time leaving just an integer 
comparison at run time. Down side is getting access to the original 
strings during debugging/runtime. This could be overcome by having the 
debugger/runtime load up the mapping of hashed values to original strings.


So given the above example we'd be able to have something like this 
(name of macro pending):


if (e->type() == NodeUpdated) {
const QScenePropertyChangePtr  = 
qSharedPointerCast(e);

switch (propertyChange->propertyNameStringId()) {
case qStrId("scale3D"):
m_scale = propertyChange->value().value();
updateMatrix();
break;
case qStrId("rotation"):
m_rotation = propertyChange->value().value();
updateMatrix();
break;
case qStrId("translation"):
m_translation = propertyChange->value().value();
updateMatrix();
break;
case qStrId("enabled"):
m_enabled = propertyChange->value().toBool();
break;
 default:
qWarning() << "Unknown property update";
}
}

Where the qStrId macro expands to nothing and the preprocess tool 
replaces its contents with the hashed value. No idea if all of our 
supported compilers allow hooking in custom preprocessors or not.


Yes, I know in this particular case, the wrapping/unwrapping of the 
property values in QVariants is likely the bottleneck but we can solve 
that with specialised property change types.


Cheers,

Sean

--
Dr Sean Harmer | sean.har...@kdab.com | Managing Director UK
KDAB (UK) Ltd, a KDAB Group company
Tel. +44 (0)1625 809908; Sweden (HQ) +46-563-540090
Mobile: +44 (0)7545 140604
KDAB - Qt Experts

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] RFC: new moc feature

2015-12-05 Thread Olivier Goffart
On Saturday 5. December 2015 09:30:33 Sean Harmer wrote:
> Hi devs,
> 
> I'd like to get some feedback on a new feature for moc before we take it
> any further than mild musing. The context is Qt3D has some frontend
> QObject subclass types, and corresponding backend, non-QObject subclass
> types for reasons of allowing us to process data on the backend without
> blocking the main thread or locking it.
> 
> To get the property values from the frontend objects to the backend we
> wrap the data up in QScenePropertyChange and in the backend object we
> unpack the data and handle it with code something like this:
> 
>  if (e->type() == NodeUpdated) {
>  const QScenePropertyChangePtr  =
> qSharedPointerCast(e);
>  if (propertyChange->propertyName() ==
> QByteArrayLiteral("scale3D")) {
>  m_scale = propertyChange->value().value();
>  updateMatrix();
>  } else if (propertyChange->propertyName() ==
> QByteArrayLiteral("rotation")) {
>  m_rotation = propertyChange->value().value();
>  updateMatrix();
>  } else if (propertyChange->propertyName() ==
> QByteArrayLiteral("translation")) {
>  m_translation = propertyChange->value().value();
>  updateMatrix();
>  } else if (propertyChange->propertyName() ==
> QByteArrayLiteral("enabled")) {
>  m_enabled = propertyChange->value().toBool();
>  }
>  }
> 
> Not too bad in this case but those cascaded if-else if blocks are not
> good when the number of properties is large.

Why not?  Readability or performence?

> This is where the proposed new feature of moc would come in. If moc were
> able to generate an enum where each enum value corresponds to a static
> property we would be able to use a switch in the above code.
> 
> Is such a feature feasible? Are there reasons why it couldn't work?

I'm afraid this is not feasable. This enum would need to be in the header, and 
the moc generated code is not a header.


Instead, I would suggest something similar to llvm::StringSwitch
http://code.woboq.org/llvm/llvm/include/llvm/ADT/StringSwitch.h.html

-- 
Olivier 

Woboq - Qt services and support - http://woboq.com - http://code.woboq.org

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] RFC: new moc feature

2015-12-05 Thread Kevin Kofler
Olivier Goffart wrote:
> Instead, I would suggest something similar to llvm::StringSwitch
> http://code.woboq.org/llvm/llvm/include/llvm/ADT/StringSwitch.h.html

Ewww, this is horrible! It will evaluate ALL the result values passed to it, 
whether the cases match or not. That works if the values are just enum 
constants, as in their example, but if you have any function calls in it, 
the complexity and side effects (!) of EVERY function call get triggered. 
Plus, in that case, you'd be taking and storing the address of a temporary, 
which is a very very bad idea by itself. So this is a very dangerous class. 
At most, this can be used to convert the strings into an integer or enum to 
feed into a real switch that does the actual work.

Kevin Kofler

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development