Code Monkey home page Code Monkey logo

Comments (6)

pomadchin avatar pomadchin commented on July 22, 2024

Hey @paulushub thx for pointing this out, it is definitely a typo. Indeed they would be interpreted as common methods and not as constructors in this case:

val cache = CRSCache() // default one would be called, none of implemented
cache.CRSCache() // valid call
cache.CRSCache(map1, map2) // valid call

from proj4j.

paulushub avatar paulushub commented on July 22, 2024

@pomadchin Thanks for the quick response. The class needs some review.

  1. The field members crsCache and epsgCache are created when defined, so should not be created again in the first constructor.
    private ConcurrentHashMap<String, CoordinateReferenceSystem> crsCache = new ConcurrentHashMap<>();
    private ConcurrentHashMap<String, String> epsgCache = new ConcurrentHashMap<>();
  1. The uses of the computeIfAbsent method seem redundant, since the codes before calling that method already checks for the absence of the key.
    public CoordinateReferenceSystem createFromName(String name)
            throws UnsupportedParameterException, InvalidValueException, UnknownAuthorityCodeException {
        CoordinateReferenceSystem res = crsCache.get(name);
        if(res != null) return res;
        return crsCache.computeIfAbsent(name, k -> crsFactory.createFromName(name));
    }

from proj4j.

pomadchin avatar pomadchin commented on July 22, 2024

@paulushub

  1. ok, it makes some sense.
  2. I would be strong that it is not redundant. Mb semantically it seems to be redundant, but in reality computeIfAbsent looks like this (Java 8 version, and we're aiming Java 8):
    public V computeIfAbsent(K key, Function<? super K, ? extends V> mappingFunction) {
        if (key == null || mappingFunction == null)
            throw new NullPointerException();
        int h = spread(key.hashCode());
        V val = null;
        int binCount = 0;
        for (Node<K,V>[] tab = table;;) {
            Node<K,V> f; int n, i, fh;
            if (tab == null || (n = tab.length) == 0)
                tab = initTable();
            else if ((f = tabAt(tab, i = (n - 1) & h)) == null) {
                Node<K,V> r = new ReservationNode<K,V>();
                synchronized (r) {
                    if (casTabAt(tab, i, null, r)) {
                        binCount = 1;
                        Node<K,V> node = null;
                        try {
                            if ((val = mappingFunction.apply(key)) != null)
                                node = new Node<K,V>(h, key, val, null);
                        } finally {
                            setTabAt(tab, i, node);
                        }
                    }
                }
                if (binCount != 0)
                    break;
            }
            else if ((fh = f.hash) == MOVED)
                tab = helpTransfer(tab, f);
            else {
                boolean added = false;
                synchronized (f) {
                    if (tabAt(tab, i) == f) {
                        if (fh >= 0) {
                            binCount = 1;
                            for (Node<K,V> e = f;; ++binCount) {
                                K ek; V ev;
                                if (e.hash == h &&
                                    ((ek = e.key) == key ||
                                     (ek != null && key.equals(ek)))) {
                                    val = e.val;
                                    break;
                                }
                                Node<K,V> pred = e;
                                if ((e = e.next) == null) {
                                    if ((val = mappingFunction.apply(key)) != null) {
                                        added = true;
                                        pred.next = new Node<K,V>(h, key, val, null);
                                    }
                                    break;
                                }
                            }
                        }
                        else if (f instanceof TreeBin) {
                            binCount = 2;
                            TreeBin<K,V> t = (TreeBin<K,V>)f;
                            TreeNode<K,V> r, p;
                            if ((r = t.root) != null &&
                                (p = r.findTreeNode(h, key, null)) != null)
                                val = p.val;
                            else if ((val = mappingFunction.apply(key)) != null) {
                                added = true;
                                t.putTreeVal(h, key, val);
                            }
                        }
                    }
                }
                if (binCount != 0) {
                    if (binCount >= TREEIFY_THRESHOLD)
                        treeifyBin(tab, i);
                    if (!added)
                        return val;
                    break;
                }
            }
        }
        if (val != null)
            addCount(1L, binCount);
        return val;
    }

from proj4j.

pomadchin avatar pomadchin commented on July 22, 2024

@paulushub I benchmarked without crsCache.get(name) and it is a bit slower:

[info] Benchmark                       Mode  Cnt  Score   Error  Units
[info] CRSBench.getEpsgCodeTest        avgt    3  0.099 ± 0.276  us/op
[info] CRSBench.logicalEqualsFalse     avgt    3  0.043 ± 0.030  us/op
[info] CRSBench.logicalEqualsTrue      avgt    3  0.038 ± 0.040  us/op
[info] CRSBench.logicalVarEqualsFalse  avgt    3  0.043 ± 0.030  us/op
[info] CRSBench.logicalVarEqualsTrue   avgt    3  0.038 ± 0.005  us/op
[info] CRSBench.resolveCRS             avgt    3  0.064 ± 0.251  us/op

from proj4j.

paulushub avatar paulushub commented on July 22, 2024

Having used crsCache.get(name), do you still need computeIfAbsent and why?

from proj4j.

pomadchin avatar pomadchin commented on July 22, 2024

@paulushub it probably could be putIfAbsent or just put; I used computeIfAbsent to simplify signatures (Java8 has a poor syntax, and it is the only way I could use lambdas to insert values), and there would be no performance overhead in it due to the get check.

Also computeIfAbsent returns the current value associated with key, and put functions return the previous value; in other words:

ConcurrentHashMap<String, String> cache = new ConcurrentHashMap<>();

cache.putIfAbsent("str", "value"); //> returns null
cache.computeIfAbsent("str1", k -> "value"); //> returns "value"

from proj4j.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.