7
\$\begingroup\$

I tried to solve the SPOJ prime number problem below. The code produced correct results, but Time limit exceeded and also memory need around 970M which is not efficient(?)

How can I optimize this code under a time constraint of around few seconds?

Problem

Peter wants to generate some prime numbers for his cryptosystem. Help him! Your task is to generate all prime numbers between two given numbers!

Input

The input begins with the number t of test cases in a single line (t<=10). In each of the next t lines there are two numbers m and n (1 <= m <= n <= 1000000000, n-m<=100000) separated by a space.

Output

For every test case print all prime numbers p such that m <= p <= n, one number per line, test cases separated by an empty line.

#include<string.h>
#include<stdlib.h>
#include<stdio.h>
#include <cmath>
#include <vector>
#include<iostream>
using namespace std;

void simplesieve(int limit, vector<int>&prime);


void segmentedsieve() {
    int T, M, K;

    cin >> T;


    for (int t = 0; t < T; t++) {
        cin >> M >> K;
        const int limit = floor(sqrt(K)) + 1;
        vector<int> prime;
        simplesieve(K, prime);

        bool *mark = new bool[K - M];
        memset(mark, true, sizeof(bool)*(K-M));

        vector<int>::iterator it;

        for (it = prime.begin(); *it<limit; it++) {

            int lolim = floor(M / (*it))*(*it);
            if (lolim < M)
                lolim = lolim + (*it);

            for (int j = lolim; j < K; j += (*it))
            {   if(j!=*it)
                mark[j - M] = false;
            }


        }

        for (int i = M; i < K; i++) {
            if (mark[i - M] == true)
                cout << i << "\n";
        }

        delete[] mark;
    }


}



void simplesieve( int limit, vector<int>&prime) {

    bool *mark=new bool[limit + 1];

    memset(mark, true, sizeof(bool)*(limit+1));
    for (int p = 2; p*p < limit; p++) {
        if (mark[p] == true) {
            for (int i = p * 2; i < limit; i += p)
                mark[i] = false;
        }
    }
    for (int p = 2; p < limit; p++) {
        if (mark[p] == true)
        {
            prime.push_back(p);

        }

    }

}


int main()
{
    int n = 100;
    segmentedsieve();
    return 0;
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Mast Sep 27 '18 at 11:39
  • \$\begingroup\$ @Mast ok, but how can i post my updated code after got review from someone? \$\endgroup\$ – Devina Muljono Sep 27 '18 at 14:22
  • 1
    \$\begingroup\$ That is answered by the thread which Mast links in the previous comment. \$\endgroup\$ – Peter Taylor Sep 27 '18 at 14:23
5
\$\begingroup\$
  1. You are computing your prime sieve multiple times, when you only need to compute it once. You are told you will have at most 10 test cases. You could easily store those in an array, compute the maximum upper limit, and run the sieve once. Then loop through the stored test cases, and generate the output for each. Could be up to a 10x speed up.

  2. The only even prime is 2. Treat it as a special case. Then, when you are running the sieve, you can start with 3 and increment with p += 2 to skip the even numbers.

  3. Since the sieve has already eliminated all multiples of smaller prime numbers, when eliminating the multiples of p, you don’t have to start at p*2; rather you can start at p*p and skip the even multiples using i += p*2. (Consider storing p2 = p*2 to avoid the multiplication every loop iteration!)

  4. In segmentedsieve(), if the upper limit is less than largest prime your sieve produced, you don’t need to generate the second sieve; just emit the primes in the correct range.

  5. You can apply points #2 & #3 to the secondary sieve, but it will be slightly more complicated as you need to ensure you’ve calculated an odd starting multiple.

  6. To get a 8x reduction in memory usage, use a bit array. (You could get a 16x reduction, by storing flags for only odd values in the bit array, but your indexing will become more complex.)

\$\endgroup\$
  • \$\begingroup\$ thanks! i optimized the code by calling simplesieve only one time under constraint 32000 which is sqrt of 1 billion, but now it produce "wrong answer". i tested it under 100 and it produced correct prime, whats wrong with the logic(?) \$\endgroup\$ – Devina Muljono Sep 27 '18 at 11:29
  • \$\begingroup\$ If the code used to produce the correct answer, but causes [time-limit-exceeded], but now produces "wrong answer", you could generate both outputs and compare them; that would give you a clue where the error might be if the logic is wrong. Or it could simply be that you are missing "test cases separated by an empty line" in your output, and the automated tests is providing input with t > 1. \$\endgroup\$ – AJNeufeld Sep 27 '18 at 15:49
0
\$\begingroup\$

I feel that you'll probably find that using the Miller–Rabin test to check each odd number for primality, will be more performant than using a sieve.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.