[WIP] Add line by line perf (by Emilien) #3

lefessan · 2023-07-26T13:34:16Z

No description provided.

lefessan · 2023-07-31T07:28:52Z

libcob/cobperf.h

+
+typedef struct linedata linedata;
+
+typedef struct rundata rundata;


These two types are not used in the interface, what is the point of making them public ?

lefessan · 2023-07-31T07:30:20Z

libcob/cobperf.h

+   along with GnuCOBOL.  If not, see <https://www.gnu.org/licenses/>.
+*/
+
+#ifndef cobperf_h


Given the size of this file, I don't think it's worth having its own .h files (unless we start having .h files for every .c, which would actually be a good thing).

lefessan · 2023-07-31T07:31:49Z

libcob/cobperf.h

+
+typedef struct rundata rundata;
+
+void cobperf_init(void);


should be COB_EXPIMP void cob_perf_init (void); :

All prototypes in .h must have COB_EXPIMP

All extern functions of libcob should start with cob_

Always add a space before ( for function protos and calls

lefessan · 2023-07-31T07:32:02Z

libcob/cobperf.h

+ * @param section the section name
+ * @param paragraph the paragraph name
+ */
+void cobperf_runline(const char *filename, unsigned line_number, const char* section, const char* paragraph);


same as above

I would prefer cob_perf_run_line

lefessan · 2023-07-31T07:32:19Z

libcob/cobperf.h

+/**
+ * This function will save all the logs to the log file.
+ */
+void cobperf_end(void);


same as above

lefessan · 2023-07-31T07:39:29Z

cobc/typeck.c

@@ -13970,6 +13970,7 @@ cb_emit_start (cb_tree file, cb_tree op, cb_tree key, cb_tree keylen)
 void
 cb_emit_stop_run (cb_tree x)
 {
+    cb_emit (CB_BUILD_FUNCALL_0 ("cobperf_end"));


Above, it was output_line("cobperf_end();");, why use a different style ?

lefessan · 2023-07-31T07:40:27Z

libcob/cobperf.c

+    void **old_data = arr->data;
+    arr->capacity *= ARRAY_GROW_FACTOR;
+    arr->data = (void**)calloc(arr->capacity, sizeof(void*));
+    for (size_t i = 0; i < arr->size; i++) {


Use memcpy

lefessan · 2023-07-31T07:51:34Z

libcob/cobperf.c

+        run->count = 1;
+        run->total_time = timediff;
+        run->line_data = *data;
+        array_push(arr, (void *)run);


It means that the array increases every time a line is executed ? It sounds quite expensive, no ?

I think the design/specification should be written first. Typically, I see two major flaws in this implementation:

I am not sure of what we are measuring. It looks like the time spent between two lines, but I am not completely sure that we are not going to miss some changes in control flow, i.e. measuring time that was actually spent in another line ;

We cannot have one record per executed line, it should be at most one record per source line: times should accumulated, i.e. total_time should be the sum of all executions for a line (and we would get the average by dividing by count)

lefessan · 2023-07-31T07:52:39Z

libcob/cobperf.c

+    FILE *f = (FILE*)file;
+    rundata *rdata = (rundata*)data;
+
+    fprintf(f, "%s, %s, %s, %u ,%u, %lld\n",


Remove the spaces after the commas. File should be easy to parse, i.e. use either commas or spaces to separate items, not both.

lefessan · 2023-07-31T07:55:21Z

libcob/cobperf.c

+    rdata->line_data = *data;
+    array_push(arr, (void *)rdata);
+
+    FILE *f = fopen("prof.csv", "w");


A common habit is to use multiple runs to accumulate samples. So, instead of rewriting the file, we should always append.

In a final implementation, there should be two files: one generated at compile time with the locations, one generated at run time with the timings.

emilienlemaire · 2023-07-31T15:28:22Z

This could be renamed to WIP: Add profiling

emilienlemaire · 2023-07-31T15:32:55Z

All the comments will be taken into account, I need some time to further check how the codegen works to make it work better

testsuite adjustment

libcob/version.h: bump to 3.3

emilienlemaire · 2023-09-04T13:57:13Z

@lefessan Could you review this PR, I'd also like to open it on the official repo, so that I can ask Simon if he's interested and how I could improve it

lefessan · 2023-09-05T09:35:27Z

A few improvements to do:

Add some documentation in docs/
Add tests in the testsuite by making the clock deterministic
For sections, we want both the time when the section is called, and the total time of all calls in the section, either directly or direct calls to its paragraphs

lefessan · 2023-09-08T09:16:50Z

configure.ac

+
+case "$USE_XLSX" in
+  check)
+    if test "$with_json" = yes; then


The --copy option can be used to include copybooks before parsing files, for example to perform replacements or include COBOL prototypes. The --include option can be used to include a HEADER in the generated C file, for example to perform verification of external calls combined with -fstatic-call.

libcob/fileio.c: * (indexed_keylen): signature change to directly take the keydesc * [WITH_ANY_ISAM]: refactored to ease access to keydesc and keypart * (indexed_open) [WITH_ANY_ISAM]: refactored

…(currently only for the BDB backend) cobc: * codegen.c (output_file_initialization): output the indexed file/keys collating sequence (were already present in the AST) * tree.c (validate_indexed_key_field): process postponed key collating sequences * parser.y (collating_sequence_clause, collating_sequence_clause_key): replace CB_PENDING by CB_UNFINISHED on file and key collating sequence * flag.def, tree.c, tree.h, cobc.c, parser.y: add and handle a new -fdefault-file-colseq flag to specify the default collating sequence to use for files without a collating sequence clause libcob: * fileio.c (bdb_setkeycol, bdb_bt_compare, indexed_open, ...): take the file collating sequence into account when comparing keys * common.c, coblocal.h: rename common_cmps to cob_cmps and make it available locally

presumingly fixing win32 tests

…ol-3.x

* cobc/parser.y (screen_value_clause): replaced basic literals by literals

…ol-3.x

* cobc/parser.y: fix SEGFAULT when checking the BY VALUE arguments of a prototype with ANY LENGTH

…ol-3.x

Initial work by Emilien Lemaire <[email protected]> Additional features: * Do not check for enter/exit section: instead, sum time from paragraphs * Support for modules, CALL and ENTRY points * Support for recursive calls * Allocate virtual stack on demand instead of statically * Correct handling of EXIT PARAGRAPH code with 'goto' * Prevent CANCEL from dlclose a module during profiling * Customize CSV result file with COB_PROF_FORMAT * Customize CSV filename using $b/$f/$d/$t * Add some tests for RECURSIVE on PROGRAM-ID

* fix compile under WIN32 and systems without CLOCK_MONOTONIC / CLOCK_MONOTONIC_RAW * fix use of defined but unavailable CLOCK_MONOTONIC_RAW * minor refactoring

emilienlemaire force-pushed the perf-line branch from 731848a to 4250dd7 Compare July 26, 2023 13:38

lefessan commented Jul 31, 2023

View reviewed changes

sf-mensch added 3 commits August 22, 2023 07:38

missing commit for [r5167] - version increase

60557e8

testsuite adjustment

configure.ac: add -fstack-clash-protection to --enable-hardening[=no]

c140aaf

missing commit for [r5167] - version increase

8208aca

libcob/version.h: bump to 3.3

emilienlemaire force-pushed the perf-line branch 3 times, most recently from e80f0af to 3d522e3 Compare September 1, 2023 12:11

emilienlemaire force-pushed the perf-line branch 2 times, most recently from a274a3e to f890dd6 Compare September 4, 2023 11:39

emilienlemaire force-pushed the perf-line branch 5 times, most recently from 8206bcc to bbad458 Compare September 5, 2023 14:52

lefessan commented Sep 8, 2023

View reviewed changes

configure.ac Outdated

case "$USE_XLSX" in

check)

if test "$with_json" = yes; then

Copy link

Owner Author

lefessan Sep 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xxx!

emilienlemaire force-pushed the perf-line branch from e5dace0 to 9ca841a Compare September 8, 2023 12:39

lefessan force-pushed the perf-line branch 9 times, most recently from f09c48d to 44c6e92 Compare November 14, 2023 14:18

Merge branch 'gnucobol-3.x' into gcos4gnucobol-3.x

2ff35a3

lefessan force-pushed the perf-line branch 2 times, most recently from 5c9480d to 6201481 Compare January 31, 2024 15:45

lefessan and others added 7 commits January 31, 2024 18:06

Merge branch 'gnucobol-3.x' into gcos4gnucobol-3.x

4eece0f

fileio refactoring

300b542

libcob/fileio.c: * (indexed_keylen): signature change to directly take the keydesc * [WITH_ANY_ISAM]: refactored to ease access to keydesc and keypart * (indexed_open) [WITH_ANY_ISAM]: refactored

Merge branch 'gnucobol-3.x' into gcos4gnucobol-3.x

db7db96

follow-up to r5208 "header include"

93d5877

presumingly fixing win32 tests

Merge remote-tracking branch 'upstream/gnucobol-3.x' into gcos4gnucob…

5713357

…ol-3.x

ddeclerck force-pushed the perf-line branch 2 times, most recently from 1062bc9 to b0c82cd Compare March 12, 2024 14:32

ddeclerck and others added 4 commits March 13, 2024 13:45

Fix [bugs:#947]: VALUE ALL "-" not working in SCREEN SECTION

61479ba

* cobc/parser.y (screen_value_clause): replaced basic literals by literals

Merge remote-tracking branch 'upstream/gnucobol-3.x' into gcos4gnucob…

a2a51fd

…ol-3.x

Fix SEGFAULT in checking prototype arguments

14f0d09

* cobc/parser.y: fix SEGFAULT when checking the BY VALUE arguments of a prototype with ANY LENGTH

Merge remote-tracking branch 'upstream/gnucobol-3.x' into gcos4gnucob…

89c45a3

…ol-3.x

ddeclerck force-pushed the perf-line branch 3 times, most recently from 63fed8a to 67c02ad Compare March 17, 2024 15:33

ddeclerck force-pushed the perf-line branch 4 times, most recently from 89a3a6b to fe48125 Compare April 8, 2024 14:12

ddeclerck force-pushed the perf-line branch from fe48125 to 1687666 Compare April 8, 2024 14:17

Update profiling.c

f16d177

* fix compile under WIN32 and systems without CLOCK_MONOTONIC / CLOCK_MONOTONIC_RAW * fix use of defined but unavailable CLOCK_MONOTONIC_RAW * minor refactoring

ddeclerck force-pushed the perf-line branch 2 times, most recently from c7e5f3d to 8f9868e Compare April 9, 2024 14:26

Profiling fixes for code coverage

31d401b

ddeclerck force-pushed the perf-line branch from 8f9868e to 31d401b Compare April 9, 2024 14:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Add line by line perf (by Emilien) #3

[WIP] Add line by line perf (by Emilien) #3

lefessan commented Jul 26, 2023

lefessan Jul 31, 2023

lefessan Jul 31, 2023

lefessan Jul 31, 2023

lefessan Jul 31, 2023

lefessan Jul 31, 2023

lefessan Jul 31, 2023

lefessan Jul 31, 2023

lefessan Jul 31, 2023

lefessan Jul 31, 2023

lefessan Jul 31, 2023

lefessan Jul 31, 2023

emilienlemaire commented Jul 31, 2023

emilienlemaire commented Jul 31, 2023

emilienlemaire commented Sep 4, 2023

lefessan commented Sep 5, 2023

lefessan Sep 8, 2023


		typedef struct linedata linedata;

		typedef struct rundata rundata;

[WIP] Add line by line perf (by Emilien) #3

Are you sure you want to change the base?

[WIP] Add line by line perf (by Emilien) #3

Conversation

lefessan commented Jul 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emilienlemaire commented Jul 31, 2023

emilienlemaire commented Jul 31, 2023

emilienlemaire commented Sep 4, 2023

lefessan commented Sep 5, 2023

Choose a reason for hiding this comment