Thread: Suggestions for making my code more efficient?

  1. #1
    Registered User HelpfulPerson's Avatar
    Join Date
    Jun 2013
    Location
    Over the rainbow
    Posts
    288

    Suggestions for making my code more efficient?

    As most of you already know that are reading this, I have been working on an AI program lately. I have the code working as far as sorting and separating input from a file but I don't think that it's very efficient. My estimates with pen and paper tell me that it will approximately 7-8 seconds to load a 1,000 line file. That's without any sorting with qsort() and searching with bsearch() that I planned on using later. In the code I will post below, is there anything that would reduce the approximated time?

    Code:
    /* Preprocessor directives */
    
    
    #include <stdio.h>
    #include <string.h>
    #include <time.h>
    #include <ctype.h>
    #include <stdlib.h>
    
    
    #include <windows.h>
    
    
    /* Structures */
    
    
    typedef struct memory memory;
    
    
    struct memory
    {
        char * question;
    
    
        char ** responses;
        int MAX_RESPONSE; /* No good way to make this constant, but it shouldn't change after it is set */
    
    
        memory * next;
    
    
    };
    
    
    typedef struct tokens tokens;
    
    
    struct tokens
    {
        char * token;
    
    
        tokens * next;
    
    
    };
    
    
    /* ************************************************************************** */
    /* String functions */
    
    
    char * s_strdup( char * dest, char * string )
    {
        dest = malloc(strlen(string) + 1);
    
    
        if ( !dest )
        {
            perror("Malloc");
            exit(1);
        }
    
    
        return strcpy(dest, string);
    }
    
    
    /* ************************************************************************** */
    
    
    char * s_filter_string( char * string )
    {
        char * temp = NULL;
        int num = 0;
        int len = 0;
    
    
        for ( ; ; )
        {
            if ((temp = strchr( string, '!' )))
                *temp = '\0';
            if ((temp = strchr( string, '?' )))
                *temp = '\0';
            if ((temp = strchr( string, '.' )))
                *temp = '\0';
    
    
            if (temp == NULL )
                break;
    
    
            temp = NULL;
        }
    
    
        len = strlen(string);
    
    
        for (temp = string; num < len; num++, temp++)
            *temp = tolower(*temp);
    
    
        return string;
    }
    
    
    /* ************************************************************************** */
    /* Printing functions */
    
    
    void print_tokens_list( tokens * list )
    {
        for (; list != NULL; list = list->next )
        {
            fprintf(stdout, "\t%s\n", list->token);
        }
    
    
        putchar('\n');
    
    
        return;
    }
    
    
    /* ************************************************************************** */
    
    
    void print_brain_memory( memory * list )
    {
        int response_num = 0;
    
    
        for (; list != NULL; list = list->next )
        {
            fprintf(stdout, "\tQuestion : %s\n\n", list->question );
    
    
            for (response_num = 0; response_num < list->MAX_RESPONSE; response_num++)
            {
                fprintf(stdout, "\tResponse %d : %s\n", response_num, list->responses[response_num]);
            }
    
    
            putchar('\n');
        }
    
    
        return;
    }
    
    
    /* ************************************************************************** */
    /* Memory functions */
    
    
    tokens * append_tokens_list(tokens * head, char * token)
    {
        tokens *temp;
        tokens *newnode;
    
    
        newnode = malloc(sizeof(*head));
    
    
        if (!newnode)
        {
            perror("malloc");
            exit(1);
        }
    
    
        newnode->token = token;
        newnode->next = NULL;
    
    
        if (!head)
            head = newnode;
        else
        {
            temp = head;
            for ( ; temp->next != NULL; temp = temp->next );
            temp->next = newnode;
        }
    
    
        return head;
    }
    
    
    /* ************************************************************************** */
    
    
    memory * create_mem_node(  memory * brain )
    {
        if (!(brain = malloc(sizeof(memory))))
        {
            perror("Malloc");
            exit(1);
        }
    
    
        return brain;
    }
    
    
    /* ************************************************************************** */
    
    
    char ** fill_responses( tokens * temp, char ** responses )
    {
        int offset = 0;
    
    
        for ( ; temp != NULL; temp = temp->next, offset++ )
        {
            responses[offset] = s_strdup(responses[offset - 1], s_filter_string(temp->token));
        }
    
    
        return responses;
    }
    
    
    /* ************************************************************************** */
    
    
    memory * store_tokens_list( tokens * list, memory * brain, int token_amount )
    {
        tokens * temp = list;
        memory * m_temp = NULL;
    
    
        if (!brain)
        {
            brain = create_mem_node(brain);
    
    
            brain->question = NULL;
    
    
            if (brain)
                    brain->question = s_strdup(brain->question, s_filter_string(temp->token));
    
    
            if ((temp = temp->next))
            {
                brain->responses = malloc((token_amount - 1) * sizeof(char *));
    
    
                if (brain->responses)
                {
                    brain->MAX_RESPONSE = token_amount - 1;
                    brain->responses = fill_responses( temp, brain->responses );
                    brain->next = NULL;
                } else
                {
                    perror("Malloc");
                    exit(1);
                }
            }
        } else
        {
            for (m_temp = brain; m_temp->next != NULL; m_temp = m_temp->next);
            m_temp->next = create_mem_node(m_temp);
            m_temp = m_temp->next;
    
    
            m_temp->question = NULL;
            m_temp->question = s_strdup(m_temp->question, s_filter_string(temp->token));
    
    
            if ((temp = temp->next))
            {
                m_temp->responses = malloc((token_amount - 1) * sizeof(char *));
    
    
                if (m_temp->responses)
                {
    
    
                    m_temp->MAX_RESPONSE = token_amount - 1;
                    m_temp->responses = fill_responses( temp, m_temp->responses );
    
    
                    m_temp->next = NULL;
                } else
                {
                    perror("Malloc");
                    exit(1);
                }
            }
        }
    
    
        return brain;
    }
    
    
    /* ************************************************************************** */
    
    
    void release_brain_memory( memory * head )
    {
        int response_num = 0;
        memory * temp = NULL;
    
    
        while ( head != NULL )
        {
            for (response_num = 0; response_num < head->MAX_RESPONSE; response_num++)
            {
                free(head->responses[response_num]);
            }
    
    
            free(head->responses);
            free(head->question);
    
    
            temp = head;
            head = head->next;
            free(temp);
        }
    
    
        return;
    }
    
    
    /* ************************************************************************** */
    
    
    void release_tokens_list( tokens * head )
    {
        tokens * temp = NULL;
    
    
        while ( head != NULL )
        {
            temp = head;
            head = head->next;
            free(temp);
        }
    
    
        return;
    }
    
    
    /* ************************************************************************** */
    /* File functions */
    
    
    FILE * open_file( char * file )
    {
        FILE * temp = fopen(file, "r+");
    
    
        if (!temp)
        {
            perror("Opening file");
            getchar();
            exit(1);
        }
    
    
        return temp;
    }
    
    
    /* ************************************************************************** */
    
    
    char * get_next_file_line( FILE * file )
    {
        char * line = malloc(sizeof(char) * BUFSIZ );
        char * new_line;
    
    
        if ( !line )
        {
            perror("Malloc");
            exit(1);
        }
    
    
        if ( !fgets(line, BUFSIZ, file) )
        {
            free(line);
            return NULL;
        } else
        {
            if ((new_line = strchr(line, '\n'))) /* Removes the '\n' and replaces it with a '\0' if it is found */
                *new_line = '\0';
    
    
            return line;
        }
    }
    
    
    /* ************************************************************************** */
    
    
    tokens * parse_line( char * file_line, char * delimit, tokens * head, int * token_amount )
    {
        char * token = NULL;
    
    
        token = strtok(file_line, delimit);
    
    
        if ( token )
        {
            for (; token && !isspace(*token); *token_amount += 1 )
            {
                head = append_tokens_list(head, token);
    
    
                token = strtok(NULL, delimit);
            }
        }
    
    
        return head;
    }
    
    
    /* ************************************************************************** */
    
    
    memory * load_brain_memory(FILE * txt_file, memory * my_brain)
    {
        tokens * head = NULL;
        char * file_line = NULL;
    
    
        int array_offset = 0;
        int token_amount = 0;
    
    
        for (; (file_line = get_next_file_line(txt_file)) != NULL; array_offset++, token_amount = 0 ) /* continue the loop until an error occurs or EOF is reached */
        {
            head = parse_line( file_line, ";", head, &token_amount);
    
    
            my_brain = store_tokens_list( head, my_brain, token_amount );
            release_tokens_list( head );
    
    
            free(file_line);
    
    
            head = NULL;
            file_line = NULL;
        }
    
    
        return my_brain;
    }
    
    
    /* **************************************************************************
    * MAIN ENTRY POINT :
    */
    
    
    int main()
    {
        memory * my_brain = NULL;
    
    
        FILE * txt_file = open_file("test.txt");
    
    
        my_brain = load_brain_memory( txt_file, my_brain );
        print_brain_memory( my_brain );
        release_brain_memory( my_brain );
    
    
        return 0;
    }
    
    
    /* **************************************************************************
    * End of program.
    * **************************************************************************/
    Output :

    Code:
            Question : how are you
    
    
            Response 0 : i'm good
            Response 1 : i'm bad
            Response 2 : i'm horrible
            Response 3 : good, how are you
    
    
            Question : i'm going to kill you
    
    
            Response 0 : thank you
            Response 1 : no problem
            Response 2 : i like cows
    
    
            Question : i don't know what you are
    
    
            Response 0 : hey there
            Response 1 : i like doing things
    
    
            Question : you're the biggest noob on this planet
    
    
            Response 0 : i hate you too, but i also love you
            Response 1 : well, that escalated quickly
    
    
            Question : this is a nice text file
    
    
            Response 0 : i like random things
            Response 1 : am i getting paid for this
            Response 2 : what's my middle name
    File :

    Code:
    How are you?;I'm good.;I'm bad.;I'm horrible.;Good, how are you?
    I'm going to kill you!;Thank you!;No problem;I like cows
    I don't know what you are;Hey there!;I like doing things
    You're the biggest noob on this planet!!!!!!!!!!!!!!!!!!!!!!;I hate you too, but I also love you...;Well, that escalated quickly.
    This is a nice text file;I like random things;Am I getting paid for this?;What's my middle name?
    The input file is garbage, but I needed to try different character amounts and different formats of text, so I just typed in random things.

    I already set my compiler to the maximum program speed too.
    Last edited by HelpfulPerson; 08-10-2013 at 02:26 PM.
    "Some people think they can outsmart me, maybe. Maybe. I've yet to meet one that can outsmart bullet" - Meet the Heavy, Team Fortress 2

  2. #2
    Lurking whiteflags's Avatar
    Join Date
    Apr 2006
    Location
    United States
    Posts
    9,612
    Your current approach is to make a linked list of tokens, and then allocate an array of responses to the question that the brain actually contains. This is slow because you are switching from one data structure to another, which means a lot of time-consuming memory management and copying data around. You can change the data structure that the memory uses to store the responses and not have to do so much copying and stuff, or you can look for a smarter way to parse the file.

    When you break up this line for example:
    Code:
    How are you?;I'm good.;I'm bad.;I'm horrible.;Good, how are you?
    I don't think the tokens list is necessary. The first semi-colon marks the end of the question. The rest of the semi-colons separate the responses. On this line there are 4 semi-colons, and one marks the end of the question. With some basic math, there are 4 - 1 responses. Now you know the value of MAX_RESPONSE and how many char pointers to allocate for the array. You should be able to parse this line into a memory object. I think it's worth going through each line of the file and seeing how many semi-colons there are as the first step.

    If you get it working, then pretty much all of the code surrounding the linked list of tokens can be deleted.

    I would like to know the purpose of s_filter_string.
    Last edited by whiteflags; 08-10-2013 at 03:35 PM.

  3. #3
    Registered User HelpfulPerson's Avatar
    Join Date
    Jun 2013
    Location
    Over the rainbow
    Posts
    288
    Quote Originally Posted by whiteflags View Post
    Your current approach is to make a linked list of tokens, and then allocate an array of responses to the question that the brain actually contains. This is slow because you are switching from one data structure to another, which means a lot of time-consuming memory management and copying data around. You can change the data structure that the memory uses to store the responses and not have to do so much copying and stuff, or you can look for a smarter way to parse the file.

    When you break up this line for example:
    Code:
    How are you?;I'm good.;I'm bad.;I'm horrible.;Good, how are you?
    I don't think the tokens list is necessary. The first semi-colon marks the end of the question. The rest of the semi-colons separate the responses. On this line there are 4 semi-colons, and one marks the end of the question. With some basic math, there are 4 - 1 responses. Now you know the value of MAX_RESPONSE and how many char pointers to allocate for the array. You should be able to parse this line into a memory object. I think it's worth going through each line of the file and seeing how many semi-colons there are as the first step.

    If you get it working, then pretty much all of the code surrounding the linked list of tokens can be deleted.

    I would like to know the purpose of s_filter_string.
    The purpose of s_filter_string is to make sure a comparison can be done without worrying about different punctuation characters or character case. Honestly though, if I just made a comparison function to do that for me, then I could remove it, which I think I will do that soon. Obviously though, there's no point to comparing it just yet.
    "Some people think they can outsmart me, maybe. Maybe. I've yet to meet one that can outsmart bullet" - Meet the Heavy, Team Fortress 2

  4. #4
    Registered User
    Join Date
    Sep 2006
    Posts
    8,868
    String operations are slow. This loop:
    Code:
        for ( ; ; )
        {
            if ((temp = strchr( string, '!' )))
                *temp = '\0';
            if ((temp = strchr( string, '?' )))
                *temp = '\0';
            if ((temp = strchr( string, '.' )))
                *temp = '\0';
     
     
            if (temp == NULL )
                break;
     
             temp = NULL;
        }
     
        len = strlen(string);
      
        for (temp = string; num < len; num++, temp++)
            *temp = tolower(*temp);
    Goes through the string, several times (four). Try working through the string, just once, instead. More like:
    Code:
        for (i=0 ;string[i] ;i++ )
        {
            if (string[i]== '!' || string[i]=='?' || string[i]=='.') {
                *temp = '\0';
                string[i]=tolower(string[i]);
            }    
            temp = NULL;
        }
    //and i already is the strlen, so  
     //delete this:    len = strlen(string);
    //and you don't even need this:    len=i; 
     
    //or this
    /*
        for (temp = string; num < len; num++, temp++)
            *temp = tolower(*temp);
    */
    The fastest way to work on a string, is to traverse it as few times as is practical.

  5. #5
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,667
    Get some profiling tools, so you can see where things are really happening.
    Code:
            -:    0:Source:foo.c
            -:    0:Graph:foo.gcno
            -:    0:Data:foo.gcda
            -:    0:Runs:1
            -:    0:Programs:1
            -:    1:/* Preprocessor directives */
            -:    2:
            -:    3:
            -:    4:#include <stdio.h>
            -:    5:#include <string.h>
            -:    6:#include <time.h>
            -:    7:#include <ctype.h>
            -:    8:#include <stdlib.h>
            -:    9:
            -:   10:
            -:   11:/* Structures */
            -:   12:
            -:   13:
            -:   14:typedef struct memory memory;
            -:   15:
            -:   16:
            -:   17:struct memory
            -:   18:{
            -:   19:    char * question;
            -:   20:
            -:   21:
            -:   22:    char ** responses;
            -:   23:    int MAX_RESPONSE; /* No good way to make this constant, but it shouldn't change after it is set */
            -:   24:
            -:   25:
            -:   26:    memory * next;
            -:   27:
            -:   28:
            -:   29:};
            -:   30:
            -:   31:
            -:   32:typedef struct tokens tokens;
            -:   33:
            -:   34:
            -:   35:struct tokens
            -:   36:{
            -:   37:    char * token;
            -:   38:
            -:   39:
            -:   40:    tokens * next;
            -:   41:
            -:   42:
            -:   43:};
            -:   44:
            -:   45:
            -:   46:/* ************************************************************************** */
            -:   47:/* String functions */
            -:   48:
            -:   49:
           19:   50:char * s_strdup( char * dest, char * string )
            -:   51:{
           19:   52:    dest = malloc(strlen(string) + 1);
            -:   53:
            -:   54:
           19:   55:    if ( !dest )
            -:   56:    {
        #####:   57:        perror("Malloc");
        #####:   58:        exit(1);
            -:   59:    }
            -:   60:
            -:   61:
           19:   62:    return strcpy(dest, string);
            -:   63:}
            -:   64:
            -:   65:
            -:   66:/* ************************************************************************** */
            -:   67:
            -:   68:
           19:   69:char * s_filter_string( char * string )
            -:   70:{
           19:   71:    char * temp = NULL;
           19:   72:    int num = 0;
           19:   73:    int len = 0;
            -:   74:
            -:   75:
            -:   76:    for ( ; ; )
            -:   77:    {
           24:   78:        if ((temp = strchr( string, '!' )))
            4:   79:            *temp = '\0';
           24:   80:        if ((temp = strchr( string, '?' )))
            4:   81:            *temp = '\0';
           24:   82:        if ((temp = strchr( string, '.' )))
            5:   83:            *temp = '\0';
            -:   84:
            -:   85:
           24:   86:        if (temp == NULL )
           19:   87:            break;
            -:   88:
            -:   89:
            5:   90:        temp = NULL;
            5:   91:    }
            -:   92:
            -:   93:
           19:   94:    len = strlen(string);
            -:   95:
            -:   96:
          370:   97:    for (temp = string; num < len; num++, temp++)
          351:   98:        *temp = tolower(*temp);
            -:   99:
            -:  100:
           19:  101:    return string;
            -:  102:}
            -:  103:
            -:  104:
            -:  105:/* ************************************************************************** */
            -:  106:/* Printing functions */
            -:  107:
            -:  108:
        #####:  109:void print_tokens_list( tokens * list )
            -:  110:{
        #####:  111:    for (; list != NULL; list = list->next )
            -:  112:    {
        #####:  113:        fprintf(stdout, "\t%s\n", list->token);
            -:  114:    }
            -:  115:
            -:  116:
        #####:  117:    putchar('\n');
            -:  118:
            -:  119:
        #####:  120:    return;
            -:  121:}
            -:  122:
            -:  123:
            -:  124:/* ************************************************************************** */
            -:  125:
            -:  126:
            1:  127:void print_brain_memory( memory * list )
            -:  128:{
            1:  129:    int response_num = 0;
            -:  130:
            -:  131:
            6:  132:    for (; list != NULL; list = list->next )
            -:  133:    {
            5:  134:        fprintf(stdout, "\tQuestion : %s\n\n", list->question );
            -:  135:
            -:  136:
           19:  137:        for (response_num = 0; response_num < list->MAX_RESPONSE; response_num++)
            -:  138:        {
           14:  139:            fprintf(stdout, "\tResponse %d : %s\n", response_num, list->responses[response_num]);
            -:  140:        }
            -:  141:
            -:  142:
            5:  143:        putchar('\n');
            -:  144:    }
            -:  145:
            -:  146:
            1:  147:    return;
            -:  148:}
            -:  149:
            -:  150:
            -:  151:/* ************************************************************************** */
            -:  152:/* Memory functions */
            -:  153:
            -:  154:
           19:  155:tokens * append_tokens_list(tokens * head, char * token)
            -:  156:{
            -:  157:    tokens *temp;
            -:  158:    tokens *newnode;
            -:  159:
            -:  160:
           19:  161:    newnode = malloc(sizeof(*head));
            -:  162:
            -:  163:
           19:  164:    if (!newnode)
            -:  165:    {
        #####:  166:        perror("malloc");
        #####:  167:        exit(1);
            -:  168:    }
            -:  169:
            -:  170:
           19:  171:    newnode->token = token;
           19:  172:    newnode->next = NULL;
            -:  173:
            -:  174:
           19:  175:    if (!head)
            5:  176:        head = newnode;
            -:  177:    else
            -:  178:    {
           14:  179:        temp = head;
           14:  180:        for ( ; temp->next != NULL; temp = temp->next );
           14:  181:        temp->next = newnode;
            -:  182:    }
            -:  183:
            -:  184:
           19:  185:    return head;
            -:  186:}
            -:  187:
            -:  188:
            -:  189:/* ************************************************************************** */
            -:  190:
            -:  191:
            5:  192:memory * create_mem_node(  memory * brain )
            -:  193:{
            5:  194:    if (!(brain = malloc(sizeof(memory))))
            -:  195:    {
        #####:  196:        perror("Malloc");
        #####:  197:        exit(1);
            -:  198:    }
            -:  199:
            -:  200:
            5:  201:    return brain;
            -:  202:}
            -:  203:
            -:  204:
            -:  205:/* ************************************************************************** */
            -:  206:
            -:  207:
            5:  208:char ** fill_responses( tokens * temp, char ** responses )
            -:  209:{
            5:  210:    int offset = 0;
            -:  211:
            -:  212:
           19:  213:    for ( ; temp != NULL; temp = temp->next, offset++ )
            -:  214:    {
           14:  215:        responses[offset] = s_strdup(responses[offset - 1], s_filter_string(temp->token));
            -:  216:    }
            -:  217:
            -:  218:
            5:  219:    return responses;
            -:  220:}
            -:  221:
            -:  222:
            -:  223:/* ************************************************************************** */
            -:  224:
            -:  225:
            5:  226:memory * store_tokens_list( tokens * list, memory * brain, int token_amount )
            -:  227:{
            5:  228:    tokens * temp = list;
            5:  229:    memory * m_temp = NULL;
            -:  230:
            -:  231:
            5:  232:    if (!brain)
            -:  233:    {
            1:  234:        brain = create_mem_node(brain);
            -:  235:
            -:  236:
            1:  237:        brain->question = NULL;
            -:  238:
            -:  239:
            1:  240:        if (brain)
            1:  241:                brain->question = s_strdup(brain->question, s_filter_string(temp->token));
            -:  242:
            -:  243:
            1:  244:        if ((temp = temp->next))
            -:  245:        {
            1:  246:            brain->responses = malloc((token_amount - 1) * sizeof(char *));
            -:  247:
            -:  248:
            1:  249:            if (brain->responses)
            -:  250:            {
            1:  251:                brain->MAX_RESPONSE = token_amount - 1;
            1:  252:                brain->responses = fill_responses( temp, brain->responses );
            1:  253:                brain->next = NULL;
            -:  254:            } else
            -:  255:            {
        #####:  256:                perror("Malloc");
        #####:  257:                exit(1);
            -:  258:            }
            -:  259:        }
            -:  260:    } else
            -:  261:    {
            4:  262:        for (m_temp = brain; m_temp->next != NULL; m_temp = m_temp->next);
            4:  263:        m_temp->next = create_mem_node(m_temp);
            4:  264:        m_temp = m_temp->next;
            -:  265:
            -:  266:
            4:  267:        m_temp->question = NULL;
            4:  268:        m_temp->question = s_strdup(m_temp->question, s_filter_string(temp->token));
            -:  269:
            -:  270:
            4:  271:        if ((temp = temp->next))
            -:  272:        {
            4:  273:            m_temp->responses = malloc((token_amount - 1) * sizeof(char *));
            -:  274:
            -:  275:
            4:  276:            if (m_temp->responses)
            -:  277:            {
            -:  278:
            -:  279:
            4:  280:                m_temp->MAX_RESPONSE = token_amount - 1;
            4:  281:                m_temp->responses = fill_responses( temp, m_temp->responses );
            -:  282:
            -:  283:
            4:  284:                m_temp->next = NULL;
            -:  285:            } else
            -:  286:            {
        #####:  287:                perror("Malloc");
        #####:  288:                exit(1);
            -:  289:            }
            -:  290:        }
            -:  291:    }
            -:  292:
            -:  293:
            5:  294:    return brain;
            -:  295:}
            -:  296:
            -:  297:
            -:  298:/* ************************************************************************** */
            -:  299:
            -:  300:
            1:  301:void release_brain_memory( memory * head )
            -:  302:{
            1:  303:    int response_num = 0;
            1:  304:    memory * temp = NULL;
            -:  305:
            -:  306:
            7:  307:    while ( head != NULL )
            -:  308:    {
           19:  309:        for (response_num = 0; response_num < head->MAX_RESPONSE; response_num++)
            -:  310:        {
           14:  311:            free(head->responses[response_num]);
            -:  312:        }
            -:  313:
            -:  314:
            5:  315:        free(head->responses);
            5:  316:        free(head->question);
            -:  317:
            -:  318:
            5:  319:        temp = head;
            5:  320:        head = head->next;
            5:  321:        free(temp);
            -:  322:    }
            -:  323:
            -:  324:
            1:  325:    return;
            -:  326:}
            -:  327:
            -:  328:
            -:  329:/* ************************************************************************** */
            -:  330:
            -:  331:
            5:  332:void release_tokens_list( tokens * head )
            -:  333:{
            5:  334:    tokens * temp = NULL;
            -:  335:
            -:  336:
           29:  337:    while ( head != NULL )
            -:  338:    {
           19:  339:        temp = head;
           19:  340:        head = head->next;
           19:  341:        free(temp);
            -:  342:    }
            -:  343:
            -:  344:
            5:  345:    return;
            -:  346:}
            -:  347:
            -:  348:
            -:  349:/* ************************************************************************** */
            -:  350:/* File functions */
            -:  351:
            -:  352:
            1:  353:FILE * open_file( char * file )
            -:  354:{
            1:  355:    FILE * temp = fopen(file, "r+");
            -:  356:
            -:  357:
            1:  358:    if (!temp)
            -:  359:    {
        #####:  360:        perror("Opening file");
        #####:  361:        getchar();
        #####:  362:        exit(1);
            -:  363:    }
            -:  364:
            -:  365:
            1:  366:    return temp;
            -:  367:}
            -:  368:
            -:  369:
            -:  370:/* ************************************************************************** */
            -:  371:
            -:  372:
            6:  373:char * get_next_file_line( FILE * file )
            -:  374:{
            6:  375:    char * line = malloc(sizeof(char) * BUFSIZ );
            -:  376:    char * new_line;
            -:  377:
            -:  378:
            6:  379:    if ( !line )
            -:  380:    {
        #####:  381:        perror("Malloc");
        #####:  382:        exit(1);
            -:  383:    }
            -:  384:
            -:  385:
            6:  386:    if ( !fgets(line, BUFSIZ, file) )
            -:  387:    {
            1:  388:        free(line);
            1:  389:        return NULL;
            -:  390:    } else
            -:  391:    {
            5:  392:        if ((new_line = strchr(line, '\n'))) /* Removes the '\n' and replaces it with a '\0' if it is found */
            4:  393:            *new_line = '\0';
            -:  394:
            -:  395:
            5:  396:        return line;
            -:  397:    }
            -:  398:}
            -:  399:
            -:  400:
            -:  401:/* ************************************************************************** */
            -:  402:
            -:  403:
            5:  404:tokens * parse_line( char * file_line, char * delimit, tokens * head, int * token_amount )
            -:  405:{
            5:  406:    char * token = NULL;
            -:  407:
            -:  408:
            5:  409:    token = strtok(file_line, delimit);
            -:  410:
            -:  411:
            5:  412:    if ( token )
            -:  413:    {
           24:  414:        for (; token && !isspace(*token); *token_amount += 1 )
            -:  415:        {
           19:  416:            head = append_tokens_list(head, token);
            -:  417:
            -:  418:
           19:  419:            token = strtok(NULL, delimit);
            -:  420:        }
            -:  421:    }
            -:  422:
            -:  423:
            5:  424:    return head;
            -:  425:}
            -:  426:
            -:  427:
            -:  428:/* ************************************************************************** */
            -:  429:
            -:  430:
            1:  431:memory * load_brain_memory(FILE * txt_file, memory * my_brain)
            -:  432:{
            1:  433:    tokens * head = NULL;
            1:  434:    char * file_line = NULL;
            -:  435:
            -:  436:
            1:  437:    int array_offset = 0;
            1:  438:    int token_amount = 0;
            -:  439:
            -:  440:
            6:  441:    for (; (file_line = get_next_file_line(txt_file)) != NULL; array_offset++, token_amount = 0 ) /* continue the loop until an error occurs or EOF is reached */
            -:  442:    {
            5:  443:        head = parse_line( file_line, ";", head, &token_amount);
            -:  444:
            -:  445:
            5:  446:        my_brain = store_tokens_list( head, my_brain, token_amount );
            5:  447:        release_tokens_list( head );
            -:  448:
            -:  449:
            5:  450:        free(file_line);
            -:  451:
            -:  452:
            5:  453:        head = NULL;
            5:  454:        file_line = NULL;
            -:  455:    }
            -:  456:
            -:  457:
            1:  458:    return my_brain;
            -:  459:}
            -:  460:
            -:  461:
            -:  462:/* **************************************************************************
            -:  463:* MAIN ENTRY POINT :
            -:  464:*/
            -:  465:
            -:  466:
            1:  467:int main()
            -:  468:{
            1:  469:    memory * my_brain = NULL;
            -:  470:
            -:  471:
            1:  472:    FILE * txt_file = open_file("test.txt");
            -:  473:
            -:  474:
            1:  475:    my_brain = load_brain_memory( txt_file, my_brain );
            1:  476:    print_brain_memory( my_brain );
            1:  477:    release_brain_memory( my_brain );
            -:  478:
            -:  479:
            1:  480:    return 0;
            -:  481:}
            -:  482:
            -:  483:
            -:  484:/* **************************************************************************
            -:  485:* End of program.
            -:  486:* **************************************************************************/
    
    
    $ gcc -fprofile-arcs -ftest-coverage foo.c
    $ ./a.out 
    $ gcov foo.c
    Look at the higher numbers, and think about what you're doing.

    The lines with hashes were never executed. Some is obviously error handling code which isn't normally expected, but some other stuff is obviously dead code.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  6. #6
    Registered User HelpfulPerson's Avatar
    Join Date
    Jun 2013
    Location
    Over the rainbow
    Posts
    288
    Do the higher numbers in that profiler represent higher cost or latency issues? Also, I left the print_tokens_list() function in when I was still testing it, but I forgot to delete it.
    "Some people think they can outsmart me, maybe. Maybe. I've yet to meet one that can outsmart bullet" - Meet the Heavy, Team Fortress 2

  7. #7
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,667
    No, the numbers represent simply the number of times that particular line of code was executed.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  8. #8
    Registered User HelpfulPerson's Avatar
    Join Date
    Jun 2013
    Location
    Over the rainbow
    Posts
    288
    Quote Originally Posted by Salem View Post
    No, the numbers represent simply the number of times that particular line of code was executed.
    Then I probably should rethink the ones that were executed 300+ times.
    "Some people think they can outsmart me, maybe. Maybe. I've yet to meet one that can outsmart bullet" - Meet the Heavy, Team Fortress 2

  9. #9
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    Quote Originally Posted by HelpfulPerson View Post
    Then I probably should rethink the ones that were executed 300+ times.
    Maybe.

    It would also pay to ponder if those 300+ times are necessary. That means looking at the callers. If you can reduce the number of times those functions are called, it might not be necessary to optimise them.

    Since your code is calling malloc() in many places, and releasing this in various ways - within loops - that is a distinct possibility.
    Right 98% of the time, and don't care about the other 3%.

    If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Buttercup, and read this, this, and this before posting again.

  10. #10
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,667
    Just to confuse you even more, an actual profile result (this is with a file with 40000 lines in it, just to get some meaningful times.
    Code:
    $ gcc -Wall -pg foo.c
    $ rm gmon.out 
    $ ./a.out > tmp10k.txt
    $ gprof a.out 
    Flat profile:
    
    Each sample counts as 0.01 seconds.
      %   cumulative   self              self     total           
     time   seconds   seconds    calls   s/call   s/call  name    
     99.91      8.55     8.55    40001     0.00     0.00  store_tokens_list
      0.23      8.57     0.02   180001     0.00     0.00  s_filter_string
      0.12      8.58     0.01        1     0.01     0.01  print_brain_memory
      0.00      8.58     0.00   180001     0.00     0.00  append_tokens_list
      0.00      8.58     0.00   180001     0.00     0.00  s_strdup
      0.00      8.58     0.00    40002     0.00     0.00  get_next_file_line
      0.00      8.58     0.00    40001     0.00     0.00  create_mem_node
      0.00      8.58     0.00    40001     0.00     0.00  fill_responses
      0.00      8.58     0.00    40001     0.00     0.00  parse_line
      0.00      8.58     0.00    40001     0.00     0.00  release_tokens_list
      0.00      8.58     0.00        1     0.00     8.57  load_brain_memory
      0.00      8.58     0.00        1     0.00     0.00  open_file
      0.00      8.58     0.00        1     0.00     0.00  release_brain_memory
    
     %         the percentage of the total running time of the
    time       program used by this function.
    
    cumulative a running sum of the number of seconds accounted
     seconds   for by this function and those listed above it.
    
     self      the number of seconds accounted for by this
    seconds    function alone.  This is the major sort for this
               listing.
    
    calls      the number of times this function was invoked, if
               this function is profiled, else blank.
     
     self      the average number of milliseconds spent in this
    ms/call    function per call, if this function is profiled,
    	   else blank.
    
     total     the average number of milliseconds spent in this
    ms/call    function and its descendents per call, if this 
    	   function is profiled, else blank.
    
    name       the name of the function.  This is the minor sort
               for this listing. The index shows the location of
    	   the function in the gprof listing. If the index is
    	   in parenthesis it shows where it would appear in
    	   the gprof listing if it were to be printed.
    
    
    		     Call graph (explanation follows)
    
    
    granularity: each sample hit covers 2 byte(s) for 0.12% of 8.58 seconds
    
    index % time    self  children    called     name
                                                     <spontaneous>
    [1]    100.0    0.00    8.58                 main [1]
                    0.00    8.57       1/1           load_brain_memory [3]
                    0.01    0.00       1/1           print_brain_memory [6]
                    0.00    0.00       1/1           open_file [13]
                    0.00    0.00       1/1           release_brain_memory [14]
    -----------------------------------------------
                    8.55    0.02   40001/40001       load_brain_memory [3]
    [2]     99.9    8.55    0.02   40001         store_tokens_list [2]
                    0.00    0.02   40001/40001       fill_responses [5]
                    0.00    0.00   40001/180001      s_filter_string [4]
                    0.00    0.00   40001/40001       create_mem_node [10]
                    0.00    0.00   40001/180001      s_strdup [8]
    -----------------------------------------------
                    0.00    8.57       1/1           main [1]
    [3]     99.9    0.00    8.57       1         load_brain_memory [3]
                    8.55    0.02   40001/40001       store_tokens_list [2]
                    0.00    0.00   40002/40002       get_next_file_line [9]
                    0.00    0.00   40001/40001       parse_line [11]
                    0.00    0.00   40001/40001       release_tokens_list [12]
    -----------------------------------------------
                    0.00    0.00   40001/180001      store_tokens_list [2]
                    0.02    0.00  140000/180001      fill_responses [5]
    [4]      0.2    0.02    0.00  180001         s_filter_string [4]
    -----------------------------------------------
                    0.00    0.02   40001/40001       store_tokens_list [2]
    [5]      0.2    0.00    0.02   40001         fill_responses [5]
                    0.02    0.00  140000/180001      s_filter_string [4]
                    0.00    0.00  140000/180001      s_strdup [8]
    -----------------------------------------------
                    0.01    0.00       1/1           main [1]
    [6]      0.1    0.01    0.00       1         print_brain_memory [6]
    -----------------------------------------------
                    0.00    0.00  180001/180001      parse_line [11]
    [7]      0.0    0.00    0.00  180001         append_tokens_list [7]
    -----------------------------------------------
                    0.00    0.00   40001/180001      store_tokens_list [2]
                    0.00    0.00  140000/180001      fill_responses [5]
    [8]      0.0    0.00    0.00  180001         s_strdup [8]
    -----------------------------------------------
                    0.00    0.00   40002/40002       load_brain_memory [3]
    [9]      0.0    0.00    0.00   40002         get_next_file_line [9]
    -----------------------------------------------
                    0.00    0.00   40001/40001       store_tokens_list [2]
    [10]     0.0    0.00    0.00   40001         create_mem_node [10]
    -----------------------------------------------
                    0.00    0.00   40001/40001       load_brain_memory [3]
    [11]     0.0    0.00    0.00   40001         parse_line [11]
                    0.00    0.00  180001/180001      append_tokens_list [7]
    -----------------------------------------------
                    0.00    0.00   40001/40001       load_brain_memory [3]
    [12]     0.0    0.00    0.00   40001         release_tokens_list [12]
    -----------------------------------------------
                    0.00    0.00       1/1           main [1]
    [13]     0.0    0.00    0.00       1         open_file [13]
    -----------------------------------------------
                    0.00    0.00       1/1           main [1]
    [14]     0.0    0.00    0.00       1         release_brain_memory [14]
    -----------------------------------------------
    
     This table describes the call tree of the program, and was sorted by
     the total amount of time spent in each function and its children.
    
     Each entry in this table consists of several lines.  The line with the
     index number at the left hand margin lists the current function.
     The lines above it list the functions that called this function,
     and the lines below it list the functions this one called.
     This line lists:
         index	A unique number given to each element of the table.
    		Index numbers are sorted numerically.
    		The index number is printed next to every function name so
    		it is easier to look up where the function in the table.
    
         % time	This is the percentage of the `total' time that was spent
    		in this function and its children.  Note that due to
    		different viewpoints, functions excluded by options, etc,
    		these numbers will NOT add up to 100%.
    
         self	This is the total amount of time spent in this function.
    
         children	This is the total amount of time propagated into this
    		function by its children.
    
         called	This is the number of times the function was called.
    		If the function called itself recursively, the number
    		only includes non-recursive calls, and is followed by
    		a `+' and the number of recursive calls.
    
         name	The name of the current function.  The index number is
    		printed after it.  If the function is a member of a
    		cycle, the cycle number is printed between the
    		function's name and the index number.
    
    
     For the function's parents, the fields have the following meanings:
    
         self	This is the amount of time that was propagated directly
    		from the function into this parent.
    
         children	This is the amount of time that was propagated from
    		the function's children into this parent.
    
         called	This is the number of times this parent called the
    		function `/' the total number of times the function
    		was called.  Recursive calls to the function are not
    		included in the number after the `/'.
    
         name	This is the name of the parent.  The parent's index
    		number is printed after it.  If the parent is a
    		member of a cycle, the cycle number is printed between
    		the name and the index number.
    
     If the parents of the function cannot be determined, the word
     `<spontaneous>' is printed in the `name' field, and all the other
     fields are blank.
    
     For the function's children, the fields have the following meanings:
    
         self	This is the amount of time that was propagated directly
    		from the child into the function.
    
         children	This is the amount of time that was propagated from the
    		child's children to the function.
    
         called	This is the number of times the function called
    		this child `/' the total number of times the child
    		was called.  Recursive calls by the child are not
    		listed in the number after the `/'.
    
         name	This is the name of the child.  The child's index
    		number is printed after it.  If the child is a
    		member of a cycle, the cycle number is printed
    		between the name and the index number.
    
     If there are any cycles (circles) in the call graph, there is an
     entry for the cycle-as-a-whole.  This entry shows who called the
     cycle (as parents) and the members of the cycle (as children.)
     The `+' recursive calls entry shows the number of function calls that
     were internal to the cycle, and the calls entry for each member shows,
     for that member, how many times it was called from other members of
     the cycle.
    
    
    
    Index by function name
    
       [7] append_tokens_list     [13] open_file               [4] s_filter_string
      [10] create_mem_node        [11] parse_line              [8] s_strdup
       [5] fill_responses          [6] print_brain_memory      [2] store_tokens_list
       [9] get_next_file_line     [14] release_brain_memory
       [3] load_brain_memory      [12] release_tokens_list
    The elephant in the room is store_tokens_list, which by itself accounts for over 99% of the run-time.

    > for (m_temp = brain; m_temp->next != NULL; m_temp = m_temp->next);
    1. Do you really have to search for the end of the list to add a new node? Could you for example add a new node at the head of the list which takes constant time.
    2. If you do need to add to the end of the list, then consider a separate 'list' structure which maintains both a 'head' and a 'tail' pointer. With a maintained tail pointer, you can append to a list in constant time as well.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  11. #11
    Registered User HelpfulPerson's Avatar
    Join Date
    Jun 2013
    Location
    Over the rainbow
    Posts
    288
    Salem, what do you mean by adding a node to the head of the list? How would I do that without losing any references?

    Also, I came up with this function for counting the semicolons like suggested above. I used pseudo code someone suggested on stack overflow to come up with the characters array implementation.

    Code:
    int count_occurences(char * string, char character)
    {
        char * temp = string;
    
    
        size_t str_sz = strlen(string);
        unsigned int ct = 0;
        unsigned int characters[128];
    
    
        memset(characters, 0, sizeof(characters));
    
    
        for (; ct < str_sz; ct++, temp++)
            ++characters[(int)*temp];
    
    
        return characters[(int)character];
    }
    "Some people think they can outsmart me, maybe. Maybe. I've yet to meet one that can outsmart bullet" - Meet the Heavy, Team Fortress 2

  12. #12
    Lurking whiteflags's Avatar
    Join Date
    Apr 2006
    Location
    United States
    Posts
    9,612
    It doesn't make sense to create a whole table to count the number of occurrences of one character in a line.

    I was thinking of something much simpler, like:
    Code:
    int occurrences = 0;
    for (int i = 0; i < lengthofline; i++) {
      if (line[i] == ';') 
        ++occurrences;
    }
    
    memory.MAX_RESPONSES = occurences;
    memory.responses = malloc(memory.MAX_RESPONSES * sizeof(char*));
    Now responses is the right size, so all you need to do is store strings.

  13. #13
    Hurry Slowly vart's Avatar
    Join Date
    Oct 2006
    Location
    Rishon LeZion, Israel
    Posts
    6,788
    Quote Originally Posted by HelpfulPerson View Post

    Code:
    int count_occurences(char * string, char character)
    {
        char * temp = string;
    
    
        size_t str_sz = strlen(string);
        unsigned int ct = 0;
        unsigned int characters[128];
    
    
        memset(characters, 0, sizeof(characters));
    
    
        for (; ct < str_sz; ct++, temp++)
            ++characters[(int)*temp];
    
    
        return characters[(int)character];
    }
    In addition to the whiteflags' suggestion - I do not see a reason to scan line twice -strlen is essentially a loop - till NUL is found. So it can be easily skipped

    Code:
    for (int i = 0; string[i] != 0 ; i++) {
      if (string[i] == character)
        ++occurrences;
    }
    All problems in computer science can be solved by another level of indirection,
    except for the problem of too many layers of indirection.
    – David J. Wheeler

  14. #14
    Registered User
    Join Date
    Sep 2006
    Posts
    8,868
    Quote Originally Posted by whiteflags View Post
    It doesn't make sense to create a whole table to count the number of occurrences of one character in a line.

    I was thinking of something much simpler, like:
    Code:
    int occurrences = 0;
    for (int i = 0; i < lengthofline; i++) {
      if (line[i] == ';') 
        ++occurrences;
    }
    
    memory.MAX_RESPONSES = occurences;
    
    memory.responses = malloc(memory.MAX_RESPONSES * sizeof(char*));
    Now responses is the right size, so all you need to do is store strings.
    the variable "occurrences" isn't needed at all. Neither is the if() test, inside the for loop. Just
    Code:
    for (int i = 0; line[i] != ';'; i++) {
    }
    
    memory.MAX_RESPONSES = i;
    will do.

    But as Salem showed, the linked list issues are the real bottleneck, atm.

    If you are happy with the current "add node at the end of the list" logic, then use a new variable to keep track of where the end of the list is currently, so the list doesn't have to be "walked" to locate it, for each new node's addition.
    Last edited by Adak; 08-11-2013 at 11:16 PM.

  15. #15
    Lurking whiteflags's Avatar
    Join Date
    Apr 2006
    Location
    United States
    Posts
    9,612
    Quote Originally Posted by Adak View Post
    the variable "occurrences" isn't needed at all. Neither is the if() test, inside the for loop. Just
    Code:
    for (int i = 0; line[i] != ';'; i++) {
    }
    
    memory.MAX_RESPONSES = i;
    will do.
    I don't really understand how you came to that conclusion. I don't think you understand what I was suggesting.

    But as Salem showed, the linked list issues are the real bottleneck, atm.
    It's nice that someone agrees with me.
    Last edited by whiteflags; 08-11-2013 at 11:24 PM.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Suggestions on making this program better
    By fredz0003 in forum C++ Programming
    Replies: 7
    Last Post: 10-06-2011, 08:58 PM
  2. How to Make This code more efficient
    By Soulzityr in forum C Programming
    Replies: 9
    Last Post: 04-12-2010, 01:29 AM
  3. How efficient is this code?
    By Morgan in forum C Programming
    Replies: 6
    Last Post: 05-23-2003, 04:47 AM
  4. need some suggestions for making 3D game
    By SuperNewbie in forum Game Programming
    Replies: 4
    Last Post: 01-05-2003, 01:54 PM
  5. more efficient code
    By Unregistered in forum C++ Programming
    Replies: 1
    Last Post: 10-11-2001, 01:56 PM