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

  1. Write a merge request
  2. Review
  3. Commit

User interface changes

None

API changes

None

Data model changes

Hashes are now generated with xxHash

Issue fork domain-3574813

Command icon 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

idebr created an issue. See original summary.

mably’s picture

Status: Active » Postponed

Let's wait for its merge in core first.

idebr’s picture

mably’s picture

The only place I've found a hash() call is in the Domain entity class:

  /**
   * {@inheritdoc}
   */
  public function createDomainId() {
    // We cannot reliably use sequences (1, 2, 3) because those can be different
    // across environments. Instead, we use the crc32 hash function to create a
    // unique numeric id for each domain. In some systems (Windows?) we have
    // reports of crc32 returning a negative number. Issue #2794047.
    // If we don't use hash(), then crc32() returns different results for 32-
    // and 64-bit systems. On 32-bit systems, the number returned may also be
    // too large for PHP.
    // See #2908236.
    $id = hash('crc32', $this->id());
    $id = abs(hexdec(substr($id, 0, -2)));
    $this->createNumericId($id);
  }

It seems we could replace it by:

/**
 * {@inheritdoc}
 */
public function createDomainId() {
  // Use xxHash instead of CRC32 for better performance and distribution.
  // xxh3 returns a 64-bit hash as hex, stable across platforms.
  // We truncate and convert to a positive integer to ensure portability
  // and compatibility with numeric storage.
  $hash = hash('xxh3', $this->id());        // 64-bit hex string
  $id   = hexdec(substr($hash, 0, 15));     // fits safely in signed 64-bit
  $this->createNumericId($id);
}

@idebr is it what you meant?

idebr’s picture

The 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

mably’s picture

Could this be a better solution?

/**
 * Builds a cache key based on the path and options.
 */
protected function buildCacheKey(string $path, array $options): string {
  $normalized = $this->normalizeOptions($options);
  // Use a fast, non-cryptographic hash with excellent distribution.
  return hash(
    'xxh3',
    $path . "\0" . json_encode($normalized, JSON_UNESCAPED_SLASHES)
  );
}

Looks like using serialize() was even more costly than using md5() here.

idebr’s picture

That looks good to me

mably’s picture

Status: Postponed » Needs review

Switched back to serialize() as it was consistently faster (slightly) after running a few benchmarks.

Can we have an RTBC please?

  • mably committed 79f6f919 on 3.x
    task: #3574813 Implement xxHash for non-cryptographic use-cases
    
    By:...
mably’s picture

Status: Needs review » Fixed

Merging as the xxh3 version is almost twice as fast as the md5 one.

Thanks @idebr!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.