0
import net.openhft.chronicle.map.ChronicleMap;

import java.io.File;
import java.io.Serializable;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

public class App {

    public static void main(String[] args) throws Exception  {

        Map<Point, Point> map1 = new ConcurrentHashMap<>();

        ChronicleMap<Point, Point> map2 = ChronicleMap
                .of(Point.class, Point.class)
                .name("map")
                .averageKey(new Point(10, 10))
                .averageValue(new Point(10, 10))
                .entries(50)
                .createPersistedTo(new File("c:/temp/map/param.dat"));

        Point key = new Point(12, 12);
        key.hashCode();

        map1.put(key, key);
        map2.put(key, key);

        System.out.println("ConcurrentHashMap.get returned " + map1.get(new Point(12, 12)));
        System.out.println("ChronicleMap.get returned " + map2.get(new Point(12, 12)));
    }
}

class Point implements Serializable {
    private int x = 0;
    private int y = 0;
    private int _hash = 0;

    public Point(int x, int y) {
        this.x = x;
        this.y = y;
    }

    public int getX() {
        return x;
    }

    public int getY() {
        return y;
    }


    @Override
    public String toString() {
        return super.toString() +  " {" + x + "," + y + "}";
    }

    @Override
    public int hashCode() {
        _hash = 1;
        _hash = _hash * 17 + x;
        _hash = _hash * 31 + y;
        return _hash;
    }

    @Override
    public boolean equals(Object obj) {
        if(obj instanceof  Point) {
            return (x == ((Point) obj).getX()) && (y == ((Point) obj).getY());
        }
        return false;
    }
}

As you can see in the example above, ChronicleMap behaviour is a bit different from ConcurrentHashMap (same with HashMap) as it can't lookup keys with hashCode or equals.

can someone pinpoint what can be done to fix this ?

UPDATE; When executed, the program will return the following results:

ConcurrentHashMap.get returned App.Point@38f {12,12}
ChronicleMap.get returned null
helloWorld998
  • 9
  • 2
  • *Unrelated:* Why is `_hash` a field (aka an instance variable)? It should be just a *local variable*. – Andreas Dec 29 '16 at 21:50
  • *"As you can see in the example above, ChronicleMap behaviour is a bit different"* I must be going blind, because I can't see that. *Where* in the question would we see that? Are we supposed to see that in the output of the code? If so, edit the question and show the output, otherwise we cannot see it. – Andreas Dec 29 '16 at 21:52
  • I have updated the example with the result of the execution. the – helloWorld998 Dec 29 '16 at 22:16

1 Answers1

3

ChronicleMap serialises the key and takes a 64-bit hash of the bytes.

A 64-bit hash is used as the map is designed for a very large number of keys eg. billions, whereas a 32-bit hash tends to have a high collision rate when you have millions of keys.

It can also use more advanced hashing strategies such as these https://github.com/OpenHFT/Zero-Allocation-Hashing

Note: Using Serializable is the least efficient way to do this but never the less is fine for this example.

Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
  • 2
    And to clarify, the problem is that `_hash` is a *field* that is initially `0`, so it is 0 for the `get()` calls, but non-zero for the `put()` calls (since `hashCode()` was called), and hence the keys are not the same. **Solution:** Remove the field, it shouldn't be there, since it should just be a local variable in the `hashCode()` method. – Andreas Dec 29 '16 at 21:59
  • 1
    @Andreas If it was a cached value it could be made `transient` though making it a local variable would be best. – Peter Lawrey Dec 29 '16 at 22:01
  • 2
    True, but that would only fix the serialization issue, not the concurrency issue. If not for `_hash` field, the class is a nice well-behaved immutable object. Even though `x` and `y` are not *declared* `final`, they are *effectively* final. – Andreas Dec 29 '16 at 22:03
  • @Andreas If used properly (transient field caching the hash and computed when zero) like in `String.hashCode`, then there'd no concurrency issue (apart from totally harmless races with multiple threads computing the same value). – maaartinus Jan 07 '18 at 01:44
  • 1
    @maaartinus Sure, if used *properly*. It certainly is not used properly here, even if it was made `transient`, and my comment is for the shown code, not some entirely different implementation of `hashCode()`. All I said was that simply adding `transient` would not fix the code. Since the hash calculation here is very simple, caching the hash value is unnecessary, and I stand by my original comment: Remove the `_hash` field. – Andreas Jan 07 '18 at 01:56
  • @Andreas I see, I was commenting on what I though the OP was trying to do, not what they did. You're perfectly right! – maaartinus Jan 07 '18 at 14:24