Deserialize pickles in a asyncio executor + converted db functions reader() and IWriter.serialize() to async

[masipcat]

I measured that big objects >50kB take a lot of time to deserialize, about 50, 100, 200ms depending on the size.. This change avoids blocking all requests in the process during deserialization.

I'm not sure if we should consider this a breaking change or if it's ok as a patch version

[codecov-commenter]

Codecov Report

Merging #1171 (7c1b5d2) into master (ee38eea) will increase coverage by 0.1%.
The diff coverage is 96.6%.

@@           Coverage Diff            @@
##           master   #1171     +/-   ##
========================================
+ Coverage    94.6%   94.6%   +0.1%     
========================================
  Files         377     377             
  Lines       32650   32658      +8     
========================================
+ Hits        30873   30881      +8     
  Misses       1777    1777             

Impacted Files	Coverage Δ
guillotina/_settings.py	100.0% <ø> (ø)
guillotina/db/transaction.py	89.8% <85.8%> (ø)
guillotina/db/reader.py	100.0% <100.0%> (ø)
guillotina/db/storages/cockroach.py	80.0% <100.0%> (ø)
guillotina/db/storages/dummy.py	94.0% <100.0%> (ø)
guillotina/db/storages/pg.py	85.9% <100.0%> (ø)
guillotina/db/writer.py	95.4% <100.0%> (ø)
guillotina/factory/content.py	83.3% <100.0%> (ø)
guillotina/tests/cache/test_cache_memory.py	100.0% <100.0%> (ø)
guillotina/tests/mocks.py	85.4% <100.0%> (ø)
... and 2 more

[vangheem]

I've always thought about this.

Making the reader interface async will also allow for more flexibility if you need to hook into anything else during object reading.

async is like an infection though -- the number of interfaces this affects is a shame...

IMO, this is a breaking change. Lots of interfaces that are potentially used.

We could make the interface optionally async. Use https://github.com/plone/guillotina/blob/master/guillotina/utils/misc.py#L126 to not cause breaking change

[vangheem]

Does this need to change?

[masipcat]

I didn't add the same logic as in the deserialization function (because it's expensive to know if a python object it's big or small) but I changed the function to async just in case someone wants to use a custom logic to decide when to run_async. For example:

async def serialize(obj):
    if isinstance(obj, BigObject):
        return await run_async(pickle.dumps, obj)
    else:
        return pickle.dumps(obj)

[masipcat]

But if we feel it's not necessary I can revert this part

[masipcat]

IMO, this is a breaking change. Lots of interfaces that are potentially used.

OK, agree

We could make the interface optionally async. Use https://github.com/plone/guillotina/blob/master/guillotina/utils/misc.py#L126 to not cause breaking change

I'm ok with this change but third-party applications using the guillotina IWriter will still break so maybe we should update the minor version?

[vangheem]

maybe we should update the minor version

Yes, I agree.