|
Al 03/09/13 09:33, En/na Anand Avati ha
escrit:
Yes, I'm thinking to something similar to dict_set() (by the way, I would remove the dict_add() function). What you propose would be the simplest solution right now. However I think it would be interesting to change the return value to an error code (this would supply more detailed information in case of failure and we could use EEXIST to know if the value already existed. In fact I think it would be interesting to progressively change the -1 return code of many functions by an error code). The pointer to pointer argument could be NULL if the previous value is not needed. Of course this would change the function signature, breaking a lot of existing code. Another possibility could be to create a dict_replace() function, and possibly make it to fail if the value didn't exist. Having a data_pair_t could help to navigate from an existing element (getting next or previous. This is really interesting if dict where implemented using a sorted structure like a trie since it would allow to process a set of similar entries very fast, like the trusted.afr.<brick> values for example) or removing or replacing it without needing another lookup (a more detailed analysis would be needed to see how to handle race conditions). By the way, is really the dict_t structure used concurrently ? I haven't analyzed all the code deeply, but it seems to me that every dict_t is only accessed from a single place at once. I know. The current implementation wastes a lot of memory because it uses an array of 256 pointers, and in some places it needs to traverse the array. Not a b¡g deal, but if it is made many times it could be noticeable. In my test I used a trie with 4 child pointers (with collapsing single-child nodes) that runs a bit faster than the 256 implementation and uses much less memory. I tried with 2, 4, 16 and 256 childs per node, and 4 seems to be the best (at least for dictionary structures) though there are very little difference between 4 and 16 in terms of speed. I agree that it is not good to maintain two implementations of the same thing. Maybe we could change the trie implementation. It should be transparent. Maybe we could create a differently named macro to implement this feature and allow the developers to slowly change it. The old implementation could be flagged as deprecated and use the new one for new code. Old code will have enough time to change it until eventually the old implementation is removed. If we make important changes to the dict_t structure, we could replace current functions by macros that use the new implementation but simulates the old behavior. Yes, it is quite similar though I should analyze it more deeply. I would also try to remove some unused/unneeded fields that are used in very few places, add complexity and can be replaced easily, like extra_free and extra_stdfree in dict_t for example.
|
