Skip to main content
Commonmark migration
Source Link

#Repeating work already done#

Repeating work already done

The biggest problem with your solution is that for each test case, you start over from scratch. This causes you to do up to 10x the work that you need to do. What you should do instead is:

  1. Read all test cases first (into a vector for example).
  2. Find the maximum n of all the test cases.
  3. Use your sieve once to find all primes up to the maximum number.
  4. Using the vector of primes that you produced with the sieve, solve all test cases.

#Don't use expensive operations in loop iterator#

Don't use expensive operations in loop iterator

Depending on your compiler, this line could be costly:

    for (ll i = 2; i <= sqrt(n); ++i) {

You could instead do:

     for (ll i = 2, sqrt_n = sqrt(n); i <= sqrt_n; ++i) {

to make sure that you don't call sqrt() once per loop iteration.

#Sieve loop improvements#

Sieve loop improvements

This sieve loop:

            for (ll j = 2; (i*j) <= n; ++j) {
                primes[i*j] = 1;
            }

Could be written more optimally as:

            for (ll j = i*i, dj = i+i; j <= n; j += dj) {
                primes[j] = 1;
            }

Notice, the loop starts at i*i instead of 2*i, and it iterates by 2*i instead of by i. It also avoids multiplication within the main loop. However, this loop assumes that all even numbers have already been marked from the primes array, so you will have to do that separately when you create the primes array.

#Printing loop#

Printing loop

The printing loop is not optimal because it starts at 2, even though first may be very large:

    for (ll i = 2; i <= n; ++i) {
        if (primes[i] == 0 && i >= first) {
            cout << i << '\n';
        }
    }

You should just start the loop at first:

    for (ll i = first; i <= n; ++i) {
        if (primes[i] == 0) {
            cout << i << '\n';
        }
    }

#Repeating work already done#

The biggest problem with your solution is that for each test case, you start over from scratch. This causes you to do up to 10x the work that you need to do. What you should do instead is:

  1. Read all test cases first (into a vector for example).
  2. Find the maximum n of all the test cases.
  3. Use your sieve once to find all primes up to the maximum number.
  4. Using the vector of primes that you produced with the sieve, solve all test cases.

#Don't use expensive operations in loop iterator#

Depending on your compiler, this line could be costly:

    for (ll i = 2; i <= sqrt(n); ++i) {

You could instead do:

     for (ll i = 2, sqrt_n = sqrt(n); i <= sqrt_n; ++i) {

to make sure that you don't call sqrt() once per loop iteration.

#Sieve loop improvements#

This sieve loop:

            for (ll j = 2; (i*j) <= n; ++j) {
                primes[i*j] = 1;
            }

Could be written more optimally as:

            for (ll j = i*i, dj = i+i; j <= n; j += dj) {
                primes[j] = 1;
            }

Notice, the loop starts at i*i instead of 2*i, and it iterates by 2*i instead of by i. It also avoids multiplication within the main loop. However, this loop assumes that all even numbers have already been marked from the primes array, so you will have to do that separately when you create the primes array.

#Printing loop#

The printing loop is not optimal because it starts at 2, even though first may be very large:

    for (ll i = 2; i <= n; ++i) {
        if (primes[i] == 0 && i >= first) {
            cout << i << '\n';
        }
    }

You should just start the loop at first:

    for (ll i = first; i <= n; ++i) {
        if (primes[i] == 0) {
            cout << i << '\n';
        }
    }

Repeating work already done

The biggest problem with your solution is that for each test case, you start over from scratch. This causes you to do up to 10x the work that you need to do. What you should do instead is:

  1. Read all test cases first (into a vector for example).
  2. Find the maximum n of all the test cases.
  3. Use your sieve once to find all primes up to the maximum number.
  4. Using the vector of primes that you produced with the sieve, solve all test cases.

Don't use expensive operations in loop iterator

Depending on your compiler, this line could be costly:

    for (ll i = 2; i <= sqrt(n); ++i) {

You could instead do:

     for (ll i = 2, sqrt_n = sqrt(n); i <= sqrt_n; ++i) {

to make sure that you don't call sqrt() once per loop iteration.

Sieve loop improvements

This sieve loop:

            for (ll j = 2; (i*j) <= n; ++j) {
                primes[i*j] = 1;
            }

Could be written more optimally as:

            for (ll j = i*i, dj = i+i; j <= n; j += dj) {
                primes[j] = 1;
            }

Notice, the loop starts at i*i instead of 2*i, and it iterates by 2*i instead of by i. It also avoids multiplication within the main loop. However, this loop assumes that all even numbers have already been marked from the primes array, so you will have to do that separately when you create the primes array.

Printing loop

The printing loop is not optimal because it starts at 2, even though first may be very large:

    for (ll i = 2; i <= n; ++i) {
        if (primes[i] == 0 && i >= first) {
            cout << i << '\n';
        }
    }

You should just start the loop at first:

    for (ll i = first; i <= n; ++i) {
        if (primes[i] == 0) {
            cout << i << '\n';
        }
    }
deleted 43 characters in body
Source Link
JS1
  • 28.6k
  • 3
  • 41
  • 83
     for (ll i = 2, sqrt_n = sqrt(n);
     for (ll i = 2; i <= sqrt_n; ++i) {
            ll delta_j = i+i;
            for (ll j = i*i;i*i, dj = i+i; j <= n; j += delta_jdj) {
                primes[j] = 1;
            }
     ll sqrt_n = sqrt(n);
     for (ll i = 2; i <= sqrt_n; ++i) {
            ll delta_j = i+i;
            for (ll j = i*i; j <= n; j += delta_j) {
                primes[j] = 1;
            }
     for (ll i = 2, sqrt_n = sqrt(n); i <= sqrt_n; ++i) {
            for (ll j = i*i, dj = i+i; j <= n; j += dj) {
                primes[j] = 1;
            }
Source Link
JS1
  • 28.6k
  • 3
  • 41
  • 83

#Repeating work already done#

The biggest problem with your solution is that for each test case, you start over from scratch. This causes you to do up to 10x the work that you need to do. What you should do instead is:

  1. Read all test cases first (into a vector for example).
  2. Find the maximum n of all the test cases.
  3. Use your sieve once to find all primes up to the maximum number.
  4. Using the vector of primes that you produced with the sieve, solve all test cases.

#Don't use expensive operations in loop iterator#

Depending on your compiler, this line could be costly:

    for (ll i = 2; i <= sqrt(n); ++i) {

You could instead do:

     ll sqrt_n = sqrt(n);
     for (ll i = 2; i <= sqrt_n; ++i) {

to make sure that you don't call sqrt() once per loop iteration.

#Sieve loop improvements#

This sieve loop:

            for (ll j = 2; (i*j) <= n; ++j) {
                primes[i*j] = 1;
            }

Could be written more optimally as:

            ll delta_j = i+i;
            for (ll j = i*i; j <= n; j += delta_j) {
                primes[j] = 1;
            }

Notice, the loop starts at i*i instead of 2*i, and it iterates by 2*i instead of by i. It also avoids multiplication within the main loop. However, this loop assumes that all even numbers have already been marked from the primes array, so you will have to do that separately when you create the primes array.

#Printing loop#

The printing loop is not optimal because it starts at 2, even though first may be very large:

    for (ll i = 2; i <= n; ++i) {
        if (primes[i] == 0 && i >= first) {
            cout << i << '\n';
        }
    }

You should just start the loop at first:

    for (ll i = first; i <= n; ++i) {
        if (primes[i] == 0) {
            cout << i << '\n';
        }
    }