Problem/Motivation
The project currently uses various hashing algoritmes. PHP 8.1 adds support for xxHash, which is superior to the current implementations. See https://php.watch/versions/8.1/xxHash
Proposed resolution
Similar to Drupal Core: #3307718: Implement xxHash for non-cryptographic use-cases
Remaining tasks
- Write a merge request
- Review
- Commit
User interface changes
None
API changes
None
Data model changes
Hashes are now generated with xxHash
Issue fork domain-3574813
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
mably commentedLet's wait for its merge in core first.
Comment #3
idebr commentedFor what it's worth, Core is already using xxHash in various places:
#2531564: Fix leaky and brittle container serialization solution
#3370828: Ensure that edge caches are busted on deployments for css/js aggregates
#3307718: Implement xxHash for non-cryptographic use-cases is the issue to retro-fit xxHash
Comment #4
mably commentedThe only place I've found a
hash()call is in theDomainentity class:It seems we could replace it by:
@idebr is it what you meant?
Comment #5
idebr commentedThe use of md5 was my trigger to open an issue:
https://git.drupalcode.org/project/domain/-/blob/3.x/domain_source/src/H...
Use of md5 is often reported in audits. In this case, xxHash is the superior option.
Not sure about \Drupal\domain\Entity\Domain::createDomainId(), random_int might be a better fit
Comment #6
mably commentedCould this be a better solution?
Looks like using
serialize()was even more costly than usingmd5()here.Comment #7
idebr commentedThat looks good to me
Comment #9
mably commentedSwitched back to
serialize()as it was consistently faster (slightly) after running a few benchmarks.Can we have an RTBC please?
Comment #11
mably commentedMerging as the
xxh3version is almost twice as fast as themd5one.Thanks @idebr!