many improvements to the config facility and usage #519
Conversation
This code was way too complicated and suboptimal.
Doing strlen() over and over is extremely unefficient.
The initial config can be greatly simplified and be made more compact. Before this change: text data bss dec hex filename 1530 0 0 1530 5fa conf.o After this change: text data bss dec hex filename 1036 0 0 1036 40c conf.o
This is not used anywhere. And there is no need to have a full dictionary for that either. If necessary the original value could still be retrieved from the default-config array.
This will allow for not doing atoi() over and over everywhere.
Now that we store both the string and the numeric value for every config entries, let's grab those values directly. This should save quite some processing cycles. Those changes were performed with the following command: sed -e 's/atoi(get_conf_value *(\([^)]*\)))/get_conf_int(\1)/g' -i *.c *.y
Those didn't match the sed formula.
Similar to the previous patch. Here we use get_int() directly.
|
Thank you! Hope this wont break anything. |
|
I got this warning btw: |
|
On Sun, 21 Mar 2021, Andrés M. wrote:
Question: why you decided to improve this, being improvements on other parts more needed?
Because that was in the way of something else I want to change and
looking at that code made me cringe.
I didn't look at most of the code though. Where is more improvements
needed?
|
|
On Sun, 21 Mar 2021, Andrés M. wrote:
conf.c:110:18: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
110 | char *line = default_config;
| ^~~~~~~~~~~~~~
I'll fix it.
|
|
Well, the main bug is with circular references, some of them makes sc-im crash. For instance if you set A0 to A0+A1 message "Circular reference shows up" Old SC used to evaluate the entire table on each change..doesnt matter if your change affect or not other cells. that was changed to a dependency graph. Probably this validation should be handled in eval function of interp.c |
|
Well, the main bug is with circular references, some of them makes sc-im crash.
Right. I reported #475 already.
That code is much less obvious.
|
|
@npitre Yes. There is a problem with parse_str of dictionary.c |
|
@npitre I made an urgent commit to recover that. Please give it a look. The parsing was not working properly. |
|
`:color type=HEADINGS fg=BLACK bg=YELLOW bold=1`
command doesnt work anymore.
#526 should improve your fix. Let me know.
|
|
@npitre Latest changes seems to work ok but it seems memory alloc'ed by strdup in function 'put' of dictionary.c is not freed and makes some memory leaks ==7095== 2 bytes in 1 blocks are still reachable in loss record 1 of 296 |
|
@npitre forget about last message. I was seeing a log of what I am debugging... where I forced an exit and didnt free memory correctly |
|
@npitre forget about last message. I was seeing a log of what I am debugging... where I forced an exit and didnt free memory correctly
OK good. Because I was really puzzled.
|
Those are changes that are certainly going to improve performance
and make the code easier to understand.