@matheusbsbs / matheus-bagliano / commits / 022349c3b0

trailer: avoid unnecessary splitting on lines

trailer.c currently splits lines while processing a buffer (and also rejoins lines when needing to invoke ignore_non_trailer). Avoid such line splitting, except when generating the strings corresponding to trailers (for ease of use by clients - a subsequent patch will allow other components to obtain the layout of a trailer block in a buffer, including the trailers themselves). The main purpose of this is to make it easy to return pointers into the original buffer (for a subsequent patch), but this also significantly reduces the number of memory allocations required. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Tan committed Nov 2, 2016 at 10:29 UTC 022349c3b091f2aa047f1cd12b5409d564b25324

1 file changed +100 -94

trailer.c

+100 -94

@@ -102,12 +102,12 @@ static int same_trailer(struct trailer_item *a, struct arg_item *b)

102 return same_token(a, b) && same_value(a, b);

103 }

104

105 -static inline int contains_only_spaces(const char *str)

105 +static inline int is_blank_line(const char *str)

106 {

107 const char *s = str;

108 - while (*s && isspace(*s))

108 + while (*s && *s != '\n' && isspace(*s))

109 s++;

110 - return !*s;

110 + return !*s || *s == '\n';

111 }

112

113 static inline void strbuf_replace(struct strbuf *sb, const char *a, const char *b)

@@ -702,51 +702,71 @@ static void process_command_line_args(struct list_head *arg_head,

702 free(cl_separators);

703 }

704

705 -static struct strbuf **read_input_file(const char *file)

705 +static void read_input_file(struct strbuf *sb, const char *file)

706 {

707 - struct strbuf **lines;

708 - struct strbuf sb = STRBUF_INIT;

709 -

707 if (file) {

711 - if (strbuf_read_file(&sb, file, 0) < 0)

708 + if (strbuf_read_file(sb, file, 0) < 0)

709 die_errno(_("could not read input file '%s'"), file);

710 } else {

714 - if (strbuf_read(&sb, fileno(stdin), 0) < 0)

711 + if (strbuf_read(sb, fileno(stdin), 0) < 0)

712 die_errno(_("could not read from stdin"));

713 }

714 +}

715

718 - lines = strbuf_split(&sb, '\n');

716 +static const char *next_line(const char *str)

717 +{

718 + const char *nl = strchrnul(str, '\n');

719 + return nl + !!*nl;

720 +}

721

720 - strbuf_release(&sb);

722 +/*

723 + * Return the position of the start of the last line. If len is 0, return -1.

724 + */

725 +static int last_line(const char *buf, size_t len)

726 +{

727 + int i;

728 + if (len == 0)

729 + return -1;

730 + if (len == 1)

731 + return 0;

732 + /*

733 + * Skip the last character (in addition to the null terminator),

734 + * because if the last character is a newline, it is considered as part

735 + * of the last line anyway.

736 + */

737 + i = len - 2;

738

722 - return lines;

739 + for (; i >= 0; i--) {

740 + if (buf[i] == '\n')

741 + return i + 1;

742 + }

743 + return 0;

744 }

745

746 /*

726 - * Return the (0 based) index of the start of the patch or the line

727 - * count if there is no patch in the message.

747 + * Return the position of the start of the patch or the length of str if there

748 + * is no patch in the message.

749 */

729 -static int find_patch_start(struct strbuf **lines, int count)

750 +static int find_patch_start(const char *str)

751 {

731 - int i;

752 + const char *s;

753

733 - /* Get the start of the patch part if any */

734 - for (i = 0; i < count; i++) {

735 - if (starts_with(lines[i]->buf, "---"))

736 - return i;

754 + for (s = str; *s; s = next_line(s)) {

755 + if (starts_with(s, "---"))

756 + return s - str;

757 }

758

739 - return count;

759 + return s - str;

760 }

761

762 /*

743 - * Return the (0 based) index of the first trailer line or count if

744 - * there are no trailers. Trailers are searched only in the lines from

745 - * index (count - 1) down to index 0.

763 + * Return the position of the first trailer line or len if there are no

764 + * trailers.

765 */

747 -static int find_trailer_start(struct strbuf **lines, int count)

766 +static int find_trailer_start(const char *buf, size_t len)

767 {

749 - int start, end_of_title, only_spaces = 1;

768 + const char *s;

769 + int end_of_title, l, only_spaces = 1;

770 int recognized_prefix = 0, trailer_lines = 0, non_trailer_lines = 0;

771 /*

772 * Number of possible continuation lines encountered. This will be

@@ -758,13 +778,13 @@ static int find_trailer_start(struct strbuf **lines, int count)

778 int possible_continuation_lines = 0;

779

780 /* The first paragraph is the title and cannot be trailers */

761 - for (start = 0; start < count; start++) {

762 - if (lines[start]->buf[0] == comment_line_char)

781 + for (s = buf; s < buf + len; s = next_line(s)) {

782 + if (s[0] == comment_line_char)

783 continue;

764 - if (contains_only_spaces(lines[start]->buf))

784 + if (is_blank_line(s))

785 break;

786 }

767 - end_of_title = start;

787 + end_of_title = s - buf;

788

789 /*

790 * Get the start of the trailers by looking starting from the end for a

@@ -772,30 +792,33 @@ static int find_trailer_start(struct strbuf **lines, int count)

792 * trailers, or (ii) contains at least one Git-generated trailer and

793 * consists of at least 25% trailers.

794 */

775 - for (start = count - 1; start >= end_of_title; start--) {

795 + for (l = last_line(buf, len);

796 + l >= end_of_title;

797 + l = last_line(buf, l)) {

798 + const char *bol = buf + l;

799 const char **p;

800 int separator_pos;

801

779 - if (lines[start]->buf[0] == comment_line_char) {

802 + if (bol[0] == comment_line_char) {

803 non_trailer_lines += possible_continuation_lines;

804 possible_continuation_lines = 0;

805 continue;

806 }

784 - if (contains_only_spaces(lines[start]->buf)) {

807 + if (is_blank_line(bol)) {

808 if (only_spaces)

809 continue;

810 non_trailer_lines += possible_continuation_lines;

811 if (recognized_prefix &&

812 trailer_lines * 3 >= non_trailer_lines)

790 - return start + 1;

791 - if (trailer_lines && !non_trailer_lines)

792 - return start + 1;

793 - return count;

813 + return next_line(bol) - buf;

814 + else if (trailer_lines && !non_trailer_lines)

815 + return next_line(bol) - buf;

816 + return len;

817 }

818 only_spaces = 0;

819

820 for (p = git_generated_prefixes; *p; p++) {

798 - if (starts_with(lines[start]->buf, *p)) {

821 + if (starts_with(bol, *p)) {

822 trailer_lines++;

823 possible_continuation_lines = 0;

824 recognized_prefix = 1;

@@ -803,8 +826,8 @@ static int find_trailer_start(struct strbuf **lines, int count)

826 }

827 }

828

806 - separator_pos = find_separator(lines[start]->buf, separators);

807 - if (separator_pos >= 1 && !isspace(lines[start]->buf[0])) {

829 + separator_pos = find_separator(bol, separators);

830 + if (separator_pos >= 1 && !isspace(bol[0])) {

831 struct list_head *pos;

832

833 trailer_lines++;

@@ -814,13 +837,13 @@ static int find_trailer_start(struct strbuf **lines, int count)

837 list_for_each(pos, &conf_head) {

838 struct arg_item *item;

839 item = list_entry(pos, struct arg_item, list);

817 - if (token_matches_item(lines[start]->buf, item,

840 + if (token_matches_item(bol, item,

841 separator_pos)) {

842 recognized_prefix = 1;

843 break;

844 }

845 }

823 - } else if (isspace(lines[start]->buf[0]))

846 + } else if (isspace(bol[0]))

847 possible_continuation_lines++;

848 else {

849 non_trailer_lines++;

@@ -831,88 +854,70 @@ continue_outer_loop:

854 ;

855 }

856

834 - return count;

835 -}

836 -

837 -/* Get the index of the end of the trailers */

838 -static int find_trailer_end(struct strbuf **lines, int patch_start)

839 -{

840 - struct strbuf sb = STRBUF_INIT;

841 - int i, ignore_bytes;

842 -

843 - for (i = 0; i < patch_start; i++)

844 - strbuf_addbuf(&sb, lines[i]);

845 - ignore_bytes = ignore_non_trailer(sb.buf, sb.len);

846 - strbuf_release(&sb);

847 - for (i = patch_start - 1; i >= 0 && ignore_bytes > 0; i--)

848 - ignore_bytes -= lines[i]->len;

849 -

850 - return i + 1;

857 + return len;

858 }

859

853 -static int has_blank_line_before(struct strbuf **lines, int start)

860 +/* Return the position of the end of the trailers. */

861 +static int find_trailer_end(const char *buf, size_t len)

862 {

855 - for (;start >= 0; start--) {

856 - if (lines[start]->buf[0] == comment_line_char)

857 - continue;

858 - return contains_only_spaces(lines[start]->buf);

859 - }

860 - return 0;

863 + return len - ignore_non_trailer(buf, len);

864 }

865

863 -static void print_lines(FILE *outfile, struct strbuf **lines, int start, int end)

866 +static int ends_with_blank_line(const char *buf, size_t len)

867 {

865 - int i;

866 - for (i = start; lines[i] && i < end; i++)

867 - fprintf(outfile, "%s", lines[i]->buf);

868 + int ll = last_line(buf, len);

869 + if (ll < 0)

870 + return 0;

871 + return is_blank_line(buf + ll);

872 }

873

874 static int process_input_file(FILE *outfile,

871 - struct strbuf **lines,

875 + const char *str,

876 struct list_head *head)

877 {

874 - int count = 0;

875 - int patch_start, trailer_start, trailer_end, i;

878 + int patch_start, trailer_start, trailer_end;

879 struct strbuf tok = STRBUF_INIT;

880 struct strbuf val = STRBUF_INIT;

881 struct trailer_item *last = NULL;

882 + struct strbuf *trailer, **trailer_lines, **ptr;

883

880 - /* Get the line count */

881 - while (lines[count])

882 - count++;

883 -

884 - patch_start = find_patch_start(lines, count);

885 - trailer_end = find_trailer_end(lines, patch_start);

886 - trailer_start = find_trailer_start(lines, trailer_end);

884 + patch_start = find_patch_start(str);

885 + trailer_end = find_trailer_end(str, patch_start);

886 + trailer_start = find_trailer_start(str, trailer_end);

887

888 /* Print lines before the trailers as is */

889 - print_lines(outfile, lines, 0, trailer_start);

889 + fwrite(str, 1, trailer_start, outfile);

890

891 - if (!has_blank_line_before(lines, trailer_start - 1))

891 + if (!ends_with_blank_line(str, trailer_start))

892 fprintf(outfile, "\n");

893

894 /* Parse trailer lines */

895 - for (i = trailer_start; i < trailer_end; i++) {

895 + trailer_lines = strbuf_split_buf(str + trailer_start,

896 + trailer_end - trailer_start,

897 + '\n',

898 + 0);

899 + for (ptr = trailer_lines; *ptr; ptr++) {

900 int separator_pos;

897 - if (lines[i]->buf[0] == comment_line_char)

901 + trailer = *ptr;

902 + if (trailer->buf[0] == comment_line_char)

903 continue;

899 - if (last && isspace(lines[i]->buf[0])) {

904 + if (last && isspace(trailer->buf[0])) {

905 struct strbuf sb = STRBUF_INIT;

901 - strbuf_addf(&sb, "%s\n%s", last->value, lines[i]->buf);

906 + strbuf_addf(&sb, "%s\n%s", last->value, trailer->buf);

907 strbuf_strip_suffix(&sb, "\n");

908 free(last->value);

909 last->value = strbuf_detach(&sb, NULL);

910 continue;

911 }

907 - separator_pos = find_separator(lines[i]->buf, separators);

912 + separator_pos = find_separator(trailer->buf, separators);

913 if (separator_pos >= 1) {

909 - parse_trailer(&tok, &val, NULL, lines[i]->buf,

914 + parse_trailer(&tok, &val, NULL, trailer->buf,

915 separator_pos);

916 last = add_trailer_item(head,

917 strbuf_detach(&tok, NULL),

918 strbuf_detach(&val, NULL));

919 } else {

915 - strbuf_addbuf(&val, lines[i]);

920 + strbuf_addbuf(&val, trailer);

921 strbuf_strip_suffix(&val, "\n");

922 add_trailer_item(head,

923 NULL,

@@ -920,6 +925,7 @@ static int process_input_file(FILE *outfile,

925 last = NULL;

926 }

927 }

928 + strbuf_list_free(trailer_lines);

929

930 return trailer_end;

931 }

@@ -968,7 +974,7 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str

974 {

975 LIST_HEAD(head);

976 LIST_HEAD(arg_head);

971 - struct strbuf **lines;

977 + struct strbuf sb = STRBUF_INIT;

978 int trailer_end;

979 FILE *outfile = stdout;

980

@@ -976,13 +982,13 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str

982 git_config(git_trailer_default_config, NULL);

983 git_config(git_trailer_config, NULL);

984

979 - lines = read_input_file(file);

985 + read_input_file(&sb, file);

986

987 if (in_place)

988 outfile = create_in_place_tempfile(file);

989

990 /* Print the lines before the trailers */

985 - trailer_end = process_input_file(outfile, lines, &head);

991 + trailer_end = process_input_file(outfile, sb.buf, &head);

992

993 process_command_line_args(&arg_head, trailers);

994

@@ -993,11 +999,11 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str

999 free_all(&head);

1000

1001 /* Print the lines after the trailers as is */

996 - print_lines(outfile, lines, trailer_end, INT_MAX);

1002 + fwrite(sb.buf + trailer_end, 1, sb.len - trailer_end, outfile);

1003

1004 if (in_place)

1005 if (rename_tempfile(&trailers_tempfile, file))

1006 die_errno(_("could not rename temporary file to %s"), file);

1007

1002 - strbuf_list_free(lines);

1008 + strbuf_release(&sb);

1009 }