Skip to content
This repository has been archived by the owner on May 13, 2022. It is now read-only.

Outdated CityHash NuGet package with too slow 128-bit implementation #60

Open
adoronin opened this issue Oct 14, 2021 · 1 comment
Open

Comments

@adoronin
Copy link

adoronin commented Oct 14, 2021

It seems that System.Data.HashFunction.CityHash NuGet package is outdated. We had a performance test and the repository's version of CityHash128 (from the current master branch) is very good.
image

But the NuGet package's version of CityHash128 is very bad. It's even slower than MD5 and also consumes high memory amount:
image

I used the benchmark source code below to compare the performance and memory usage:

using System;
using System.Data.HashFunction.CityHash;
using System.Security.Cryptography;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

namespace TestCityHash
{
    class Program
    {
        static void Main(string[] args)
        {
            var summary = BenchmarkRunner.Run<CityHash128vs64vs32>();
        }
    }

    [ShortRunJob]
    [MemoryDiagnoser]
    public class CityHash128vs64vs32
    {
        private readonly byte[] _data = new byte[1000];

        private readonly MD5 _md5 = MD5.Create();
        private readonly ICityHash _cityHash128 = HashFactory.CreateCityHash128();
        private readonly ICityHash _cityHash64 = HashFactory.CreateCityHash64();
        private readonly ICityHash _cityHash32 = HashFactory.CreateCityHash32();

        public CityHash128vs64vs32()
        {
            new Random().NextBytes(_data);
        }

        [Benchmark(Baseline = true)]
        public byte[] Md5() => _md5.ComputeHash(_data);

        [Benchmark]
        public byte[] CityHash128() => _cityHash128.ComputeHash(_data).Hash;

        [Benchmark]
        public byte[] CityHash64() => _cityHash64.ComputeHash(_data).Hash;

        [Benchmark]
        public byte[] CityHash32() => _cityHash32.ComputeHash(_data).Hash;
    }

    public class HashFactory
    {
        public static ICityHash CreateCityHash128()
        {
            var config = new CityHashConfig
            {
                HashSizeInBits = 128
            };

            return CityHashFactory.Instance.Create(config);
        }

        public static ICityHash CreateCityHash64()
        {
            var config = new CityHashConfig
            {
                HashSizeInBits = 64
            };

            return CityHashFactory.Instance.Create(config);
        }

        public static ICityHash CreateCityHash32()
        {
            var config = new CityHashConfig
            {
                HashSizeInBits = 32
            };

            return CityHashFactory.Instance.Create(config);
        }
    }
}

I have compared the source code (package version vs branch version) and have to conclude that the CityHash128 implementation from the NuGet package is definitely outdated. Maybe it is possible to publish a new version of the NuGet package that contains implementation from the current master branch? E.g. from v2.0.0 to 2.0.1. That would be great. Thank you!

@adoronin
Copy link
Author

@brandondahler could you please help with that?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant