Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hash conflict in lyb_hash_siblings #2186

Open
billykinggym opened this issue Feb 26, 2024 · 4 comments
Open

hash conflict in lyb_hash_siblings #2186

billykinggym opened this issue Feb 26, 2024 · 4 comments
Labels
is:enhancement Request for adding new feature or enahncing functionality. status:wontfix Issue is real, but for some reason, it won't be fixed.

Comments

@billykinggym
Copy link

in our case, we have many many parameter in a container, so the hash is exhausted. so we have following error inside lyb_hash_siblings(struct lysc_node *sibling, struct hash_table **ht_p):

    if (i == LYB_HASH_BITS) {
        /* wow  we are here*/ 
        LOGINT(sibling->module->ctx);
        lyht_free(ht);
        return;
  }

the reason is that only 8bit hash in this case.

so we make 8bits to 16bit by the diff here
could you check if it could be merge to the trunk of libyang?

use_16bit_hash.zip

@michalvasko
Copy link
Member

We were hoping there would not be a module that would exhaust the value space, there was none until now in any case. There are good reasons for using only 8 bit hash, the data size and performance (print/parse) is best. So, my advice is to not use LYB file format if you do not have to. If you do, then you can use your patch but it will not be merged into the repo, it would slow down all use-cases.

@michalvasko michalvasko added is:enhancement Request for adding new feature or enahncing functionality. status:wontfix Issue is real, but for some reason, it won't be fixed. labels Feb 26, 2024
@billykinggym
Copy link
Author

@michalvasko thanks for you reply. but i don't understand why it will slow down use-cases if we use 16bit hash.
since if we use 16bit hash, then we will have less hash conflict, so we don't need to loop to find a unconflict collisionid, so it should be more quick . the only problme is that it will use more memory , but only 1 byte for 1 object.
currently, in our cases, we have only 500+ parameters in one object, but hash exhausted. so , is it possible for your to provide a compile option , to let me decide the length of hash (8bit or 16bit, by default it could be 8bit as it is now) . many many thanks !

@michalvasko
Copy link
Member

but i don't understand why it will slow down use-cases if we use 16bit hash.

That is fine, very common, and exactly the reason why benchmarks are performed. We measured it during the initial LYB implementation and the results were clear and I believe the reason is exactly that 1 more byte in data for every node. A compile option could be added but a much better solution would be for you to reduce the number of parameters in that one container to 256 max. So, I will not spent any time on such a patch but if you provide it in a PR and is clean enough, I would be willing to merge it.

@billykinggym
Copy link
Author

billykinggym commented Feb 27, 2024

hi, @michalvasko i think i submit the merge request #2188 could have a look

16bit_hash.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is:enhancement Request for adding new feature or enahncing functionality. status:wontfix Issue is real, but for some reason, it won't be fixed.
Projects
None yet
Development

No branches or pull requests

2 participants