Make PCFExecute.unpack() return full header

[AlexandreYang]

The header is also part of things we unpack, we should return it too. The data in the header can be as useful as the data in the message itself.

Use case: decide how to process the unpacked message depending on the header information (e.g. header.Command).

This is a breaking change since the PCFExecute.unpack() method is already released in v1.11.1, for people relying on mqcfh.Control previously returned.

Open questions:

• Any suggestion about possible way to avoid the breaking change but still get similar feature ?

cc @SeyfSV

[dsuch]

Is the unpack method something that users will commonly invoke in their code?

If it is not then I see not issues with making such a backward-incompatible change, particularly if that code has been around for only two weeks.

[SeyfSV]

I think this is not needed, because can be easily extracted from message:

        mqcfh = CFH(Version=CMQCFC.MQCFH_VERSION_1)
        mqcfh.unpack(message[:CMQCFC.MQCFH_STRUC_LENGTH])

[AlexandreYang]

I think this is not needed, because can be easily extracted from message:

        mqcfh = CFH(Version=CMQCFC.MQCFH_VERSION_1)
        mqcfh.unpack(message[:CMQCFC.MQCFH_STRUC_LENGTH])

Yes, agree it can be unpacked separately as you suggested. But that would mean unpacking twice if we want both the message and the header (not the end of the world, but avoiding extra processing is probably good).

Is the unpack method something that users will commonly invoke in their code?

Not sure about all use cases. But In the use case I have, it's yes, in order process messages from SYSTEM.ADMIN.STATISTICS.QUEUE (which can contain possibly hundreds of messages depending on the freq of the statistics messages).

[SeyfSV]

OK, it's good example.

[SeyfSV]

This is a breaking change since the PCFExecute.unpack() method is already released in v1.11.1, for people relying on mqcfh.Control previously returned.

mqcfh.Control added just for internal needs - stops messages reading , when last message arrived. I think, changing this does not break anything.

[AlexandreYang]

Thx for the fix d9a5918

[SeyfSV]

Just for info: unpacks are not our terrible foe :), but yes it has one of the biggest tottime

  python -m cProfile -s ncalls code/tests/test_pcf.py

  434152 function calls (432833 primitive calls) in 1.739 seconds

   Ordered by: call count

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
   103840    0.053    0.000    0.053    0.000 {built-in method builtins.isinstance}
    60496    0.043    0.000    0.043    0.000 {built-in method builtins.setattr}
    52678    0.061    0.000    0.087    0.000 __init__.py:166(is_unicode)
    52592    0.057    0.000    0.145    0.000 __init__.py:174(ensure_not_unicode)
    47019    0.021    0.000    0.021    0.000 {method 'append' of 'list' objects}
    18980    0.013    0.000    0.013    0.000 {built-in method builtins.getattr}
9207/8967    0.005    0.000    0.005    0.000 {built-in method builtins.len}
     6026    0.014    0.000    0.014    0.000 {built-in method _struct.unpack}
     3853    0.004    0.000    0.004    0.000 {method 'pop' of 'dict' objects}
     3839    0.009    0.000    0.009    0.000 {built-in method _struct.calcsize}
     3818    0.144    0.000    0.317    0.000 __init__.py:314(unpack)
     3818    0.010    0.000    0.018    0.000 __init__.py:395(get_length)
     3815    0.003    0.000    0.003    0.000 {method 'keys' of 'dict' objects}
     3812    0.139    0.000    0.202    0.000 __init__.py:263(__init__)
     3812    0.019    0.000    0.030    0.000 __init__.py:343(set)
     3091    0.001    0.000    0.001    0.000 {method 'rstrip' of 'str' objects}
     2000    0.001    0.000    0.001    0.000 sre_parse.py:233(__next)
     1921    0.005    0.000    0.005    0.000 {method 'format' of 'str' objects}
     1734    0.001    0.000    0.001    0.000 {method 'join' of 'str' objects}
     1602    0.004    0.000    0.004    0.000 __init__.py:185(padded_count)
     1602    0.001    0.000    0.002    0.000 sre_parse.py:254(get)

[AlexandreYang]

It seems some methods like isinstance, setattr, getattr are called by unpack. Hard to say how impactful it is. A more more realistic case would be comparing the performance for using one of the example payload :

mqcfh = CFH(Version=CMQCFC.MQCFH_VERSION_1); mqcfh.unpack(message[:CMQCFC.MQCFH_STRUC_LENGTH])
PCFExecute.unpack(message)

vs

PCFExecute.unpack(message)

---

For a large payload like statistics_q.dat, unpacking twice the header seems indeed quite small in comparison to the rest of the structure :

$ python -m timeit -n 300 --setup 'message = open("pymqi/code/tests/messages/statistics_q.dat", "rb").read(); import pymqi' 'pymqi.PCFExecute.unpack(message)'
300 loops, best of 5: 4.76 msec per loop

$ python -m timeit -n 300 --setup 'message = open("pymqi/code/tests/messages/statistics_q.dat", "rb").read(); import pymqi' 'pymqi.PCFExecute.unpack(message); mqcfh = pymqi.CFH(Version=pymqi.CMQCFC.MQCFH_VERSION_1); mqcfh.unpack(message[:pymqi.CMQCFC.MQCFH_STRUC_LENGTH])'
300 loops, best of 5: 4.84 msec per loop