Java – how to improve the performance of recursive methods?

I'm learning data structures and algorithms, which is a problem I insist on

I have to improve the performance of recursive calls by storing values in memory

But the problem is that the non - improved version seems to be faster than this

Can someone help me?

Syracuse numbers are a sequence of positive integers defined by the following rules:

syra(1)≡1

Syra (n) ≡ nsyra (n / 2), if n mod 2 = = 0

Syra (n) ≡ nsyra ((n * 3) 1), otherwise

import java.util.HashMap;
import java.util.Map;

public class SyraLengthsEfficient {

    int counter = 0;
    public int syraLength(long n) {
        if (n < 1) {
            throw new IllegalArgumentException();
        }

        if (n < 500 && map.containsKey(n)) {
            counter += map.get(n);
            return map.get(n);
        } else if (n == 1) {
            counter++;
            return 1;
        } else if (n % 2 == 0) {
            counter++;
            return syraLength(n / 2);
        } else {
            counter++;
            return syraLength(n * 3 + 1);
        }
    }

    Map<Integer,Integer> map = new HashMap<Integer,Integer>();

    public int lengths(int n) {
        if (n < 1) {
            throw new IllegalArgumentException();
        }    
        for (int i = 1; i <= n; i++) {
            syraLength(i);
            if (i < 500 && !map.containsKey(i)) {
                map.put(i,counter);
            }
        }    
        return counter;
    }

    public static void main(String[] args) {
        System.out.println(new SyraLengthsEfficient().lengths(5000000));
    }
}

This is the normal version I wrote:

public class SyraLengths{

        int total=1;
        public int syraLength(long n) {
            if (n < 1)
                throw new IllegalArgumentException();
            if (n == 1) {
                int temp=total;
                total=1;
                return temp;
            }
            else if (n % 2 == 0) {
                totaL++;
                return syraLength(n / 2);
            }
            else {
                totaL++;
                return syraLength(n * 3 + 1);
            }
        }

        public int lengths(int n){
            if(n<1){
                throw new IllegalArgumentException();
            }
            int total=0;
            for(int i=1;i<=n;i++){
                total+=syraLength(i);
            }

            return total;
        }

        public static void main(String[] args){
            System.out.println(new SyraLengths().lengths(5000000));
        }
       }

edit

It is slower than the non - enhanced version

import java.util.HashMap;
import java.util.Map;

public class SyraLengthsEfficient {

    private Map<Long,Long> map = new HashMap<Long,Long>();

    public long syraLength(long n,long count) {

        if (n < 1)
            throw new IllegalArgumentException();

        if (!map.containsKey(n)) {
            if (n == 1) {
                count++;
                map.put(n,count);
            } else if (n % 2 == 0) {
                count++;
                map.put(n,count + syraLength(n / 2,0));
            } else {
                count++;
                map.put(n,count + syraLength(3 * n + 1,0));
            }
        }

        return map.get(n);

    }

    public int lengths(int n) {
        if (n < 1) {
            throw new IllegalArgumentException();
        }
        int total = 0;
        for (int i = 1; i <= n; i++) {
            // long temp = syraLength(i,0);
            // System.out.println(i + " : " + temp);
            total += syraLength(i,0);

        }
        return total;
    }

    public static void main(String[] args) {
        System.out.println(new SyraLengthsEfficient().lengths(50000000));
    }
}

Final solution (marked as correct through the school automatic marking system)

public class SyraLengthsEfficient {

private int[] values = new int[10 * 1024 * 1024];

public int syraLength(long n,int count) {

    if (n <= values.length && values[(int) (n - 1)] != 0) {
        return count + values[(int) (n - 1)];
    } else if (n == 1) {
        count++;
        values[(int) (n - 1)] = 1;
        return count;
    } else if (n % 2 == 0) {
        count++;
        if (n <= values.length) {
            values[(int) (n - 1)] = count + syraLength(n / 2,0);
            return values[(int) (n - 1)];
        } else {
            return count + syraLength(n / 2,0);
        }
    } else {
        count++;
        if (n <= values.length) {
            values[(int) (n - 1)] = count + syraLength(n * 3 + 1,0);
            return values[(int) (n - 1)];
        } else {
            return count + syraLength(n * 3 + 1,0);
        }
    }

}

public int lengths(int n) {
    if (n < 1) {
        throw new IllegalArgumentException();
    }
    int total = 0;
    for (int i = 1; i <= n; i++) {
        total += syraLength(i,0);
    }
    return total;
}

public static void main(String[] args) {
    SyraLengthsEfficient s = new SyraLengthsEfficient();
    System.out.println(s.lengths(50000000));
}

}

Solution

Forget the answers that make your code inefficient by using map, which is not why it slows down - the fact is that you limit the cache of calculated numbers to n < 500 Once you remove this restriction, things start to get very fast; Here is the proof of concept for you to fill in the details:

private Map<Long,Long>();

public long syraLength(long n) {

    if (!map.containsKey(n)) {
        if (n == 1)
            map.put(n,1L);
        else if (n % 2 == 0)
            map.put(n,n + syraLength(n/2));
        else
            map.put(n,n + syraLength(3*n+1));
    }

    return map.get(n);

}

If you want to learn more about what's happening in the program and why it's so fast, please check out this Wikipedia article on memoization

In addition, I think you misuse the counter variable. You increase it () the first time you calculate the value, but when you find a value in the map, you accumulate it (=) This seems wrong to me. I doubt whether it gives the expected results