Optimising algorithm of self-organising sequence of numbers implemented on circular doubly linked list

Question

Few days ago I asked you in order to pick the best data structure for my problem. In that time I also explained my problem and decribed it:

Self-organising sequence of numbers with big amount of operations on it - best data structure

I have implemented it, but unfortunately it can not pass few test's. Here is my code:

#include <stdio.h>
#include <stdlib.h>
using namespace std;
int allCharCounter = 0;

struct List_node{
    int value;
    struct List_node *next;
    struct List_node *prev;
};

//inserting at first
void insert(List_node** start, int v){
    List_node* newNode = new List_node;
    newNode->value = v;

    if(*start == NULL){

        newNode->next = newNode;
        newNode->prev = newNode;
        *start = newNode;
    }else{
        newNode->next = *start;
        newNode->prev = (*start)->prev;
        (*start)->prev->next = newNode;
        (*start)->prev = newNode;
    }
}

//getting input
int getNumber(){
    int c = getchar_unlocked();
    int value = 0;
    for(; (c < 48 || c > 57); c = getchar_unlocked());

    for(; c > 47 && c < 58 ; c = getchar_unlocked()){
        value = 10*value+c-'0';
        allCharCounter++;
    }
    return value;
}


int main(){

    int numberOfOperations = getNumber();
    struct List_node* list = NULL;

    //counter of numbers
    int numbersInSeq = 0;

    //passing values to list
    while(!feof(stdin)){
        int number = getNumber();
        insert(&list, number);
        numbersInSeq++;
    }

    if(list !=NULL){

        while(numberOfOperations-- != 0){
            int c = list->value;

            //insert - X
            if(c & 1){
                List_node* newNode = new List_node;
                newNode->value = c-1;

                newNode->prev = list;
                newNode->next = list->next;
                list->next->prev = newNode;
                list->next = newNode;

                numbersInSeq++;
                int moveNext = c%numbersInSeq;
                //int movePrev = numbersInSeq - moveNext;

                for(int i = 0; i < moveNext; i++){
                    list = list->next;
                }
            }else{
                //remove - R
                c = list->next->value;
                List_node* tmp = list->next;

                list->next = tmp->next;
                list->next->prev = list;
                tmp->next = NULL;
                tmp->prev = NULL;
                free(tmp);

                numbersInSeq--;
                int moveNext = c%numbersInSeq;
                //int movePrev = numbersInSeq - moveNext;
                //moving my list (POS)
                for(int i = 0; i < moveNext; i++){
                    list = list->next;
                }
            }

        }
        //printing output
        for(int i = 0; i < numbersInSeq; i++){
            fprintf(stdout, "%d",list->value); 
            if(i != numbersInSeq-1){
                fprintf(stdout, "%c",' '); 
            }
            list = list->next;
        }

    }else{
        //in case of empty list return -1
        fprintf(stdout, "%d", -1); 
    }
    fprintf(stdout, "%c",'\n');         
    fprintf(stdout, "%d",allCharCounter);

}

This code uses circular doubly linked list, output is always correct, but as I said before it was too slow for some test's. You also may see that by mistake I implemented moving the list(POS) only using next. So I came up with this:

int moveNext = c%numbersInSeq;
int movePrev = numbersInSeq - moveNext;
if(moveNext < movePrev){
    for(int i = 0; i < moveNext; i++){
        list = list->next;                  
    }   
}else{
    for(int i = 0; i < movePrev; i++){
        list = list->prev;
    }
}

Injected right after incrementing and decrementing numbersInSeq in both X and R methods. Variable moveNext is a number of iterations that is needed to move pointer to desired place by using next. So the diffrence of it and numbersInSeq is movement by prev. Because of that I know what is more efficient, moving it using next or prev.

I have tested it on example with 50 digits and output was correct. Number of iterations was smaller:

w/o - 13001
with - 570

Not only it haven't passed even one more test there, but also it was too slow for another test(Although I don't know exactly what was in there, but I can tell you that file with its size is around 34mb).

Maybe you can see what I missed here/wrote badly/don't know about structure. Is it possible to optimise somehow my code to be faster?

If you are saying this works, but isn't fast enough, then perhaps you want https://codereview.stackexchange.com/ — Kenny Ostrom, Jan 06 '18 at 22:35

score 0 · Answer 1 · answered Jan 08 '18 at 15:28

Please review your previous question. The answer you marked as correct is incorrect in fact: all you need is a single-linked circular list, since you always traverse forward and never backward.
Obviously, operators new and delete (it is free in your case) in the code impacts performance the most. Basically, memory allocation is slow and you should avoid it in case you want to pass some performance tests.

There are variety of techniques to avoid memory allocations. The most simple would be to maintain another free_list with free list elements.

So instead of new you should write a function:

If free_list is NULL
- return new List_node // allocate
else
- n = free_list
- free_list = n->next
- return n // reuse previously allocated node

And instead of delete or free you should write a function:

node_to_free->next = free_list
free_list = node_to_free // i.e. put node to free to the free list

Those two changes should give you significant performance boost. If it will be not enough to pass the test, come for more suggestions on SO ;)

Optimising algorithm of self-organising sequence of numbers implemented on circular doubly linked list

1 Answers1