Embed pair_list_t structure into MultiDict Python object #395
Conversation
psf-chronographer
bot
added
the
bot:chronographer:provided
Codecov Report
@@ Coverage Diff @@
## master #395 +/- ##
======================================
Coverage 75.1% 75.1%
======================================
Files 5 5
Lines 470 470
======================================
Hits 353 353
Misses 117 117Continue to review full report at Codecov.
|
| uint64_t version; // 8 | ||
| calc_identity_func calc_identity; // 8 | ||
| pair_t *pairs; // 8 | ||
| } pair_list_t; |
There was a problem hiding this comment.
I think it will be nice to refactoring this name. Sufix "_t" is not so good because of POSIX use it for type definitions. Mb something like this MultiDictVec and MultiDictVecItem? But if rename structs it will be good to rename and functions too, for example, rename pair_list_* to multidict_vec_*, what do you think?
There was a problem hiding this comment.
This is the interesting question.
I've found that the separation of multidict PyObject class and pair_list_t plain C structure makes sense, at least for the current code stage.
Thinking more, pair list doesn't exist alone. It is a part of MultiDict or CIMultiDict struct. Maybe later we come up to idea of joining not only pair_list_t and MultiDict object structures but APIs also? In this case, the pair-list will go.
The pair list was crucial for the transition from Cython to pure C, I'm not sure if it still is needed.
Let's shake the code and see. If the list should stay I completely agree with MultiDictVec / MultiDictVecItem proposal. Let's decide later
It removes one indirection level.
No need for extra memalloc, plus multidict with embedded pair-list fits CPU cache a little better.