Trey Jackson <
bigfa...@gmail.com> writes:
> I disabled the use of the hash in the second case (tracking the
> provenance of strings) - and the runtime associated with
> CreateHashEntry disappeared completely.
For context. This was all done quite a few years ago (2004) for
... some customer of ActiveState. They wanted this provenance tracking
for the easier debugging of problematic scripts, i.e. easier finding
of which file the troublesome code came from, even if the code was in
some deeper eval/uplevel for the body-script of a custom-coded control
command, etc.
See TIP #280, and #378
https://core.tcl-lang.org/tips/doc/trunk/tip/280.md
https://core.tcl-lang.org/tips/doc/trunk/tip/378.md
The really big constraint was the inability to change public data
structures like Tcl_Obj in any way. No additional fields were possible
The only solution seen at the time (and seen still (to me)) was the
introduction of a hashtable keyed by the Tcl_Obj address/pointer to
hold the additional information.
The code might be more confusing than it has to be because of that.
It also pays a price in more effort required to manage all the new
information, what with having to do (Find|Create|Delete)HashEntry
instead of a simple struct field access/dereference, etc.
All the relevant places (which directly access lineCLPtr) can be found
with simple grep
hephaistos:(504) ~/Play/CoreTcl/tcl-8.7 > grep -rn lineCLPtr .
./ChangeLog:7158: [Bug 2871908]: Enforce separation of concerns between the lineCLPtr
./ChangeLog:7186: and lineCLPtr hashtables. Also make the names of the continuation
./tclInt.h:1259: * file "tclBasic.c", and stored in the thread-global hashtable "lineCLPtr" in
./tclCompile.c:809: * found in this file. The "lineCLPtr" hashtable is managed in the file
./tclBasic.c:6197: * function, after the evaluator is done. The relevant "lineCLPtr"
./tclObj.c:79: Tcl_HashTable *lineCLPtr; /* This table remembers for each Tcl_Obj
./tclObj.c:541: if (!tsdPtr->lineCLPtr) {
./tclObj.c:542: tsdPtr->lineCLPtr = ckalloc(sizeof(Tcl_HashTable));
./tclObj.c:543: Tcl_InitHashTable(tsdPtr->lineCLPtr, TCL_ONE_WORD_KEYS);
./tclObj.c:576: Tcl_CreateHashEntry(tsdPtr->lineCLPtr, objPtr, &newEntry);
./tclObj.c:731: Tcl_FindHashEntry(tsdPtr->lineCLPtr, originObjPtr);
./tclObj.c:765: Tcl_FindHashEntry(tsdPtr->lineCLPtr, objPtr);
./tclObj.c:803: for (hPtr = Tcl_FirstHashEntry(tsdPtr->lineCLPtr, &hSearch);
./tclObj.c:808: Tcl_DeleteHashTable(tsdPtr->lineCLPtr);
./tclObj.c:809: ckfree(tsdPtr->lineCLPtr);
./tclObj.c:810: tsdPtr->lineCLPtr = NULL;
./tclObj.c:1404: if (tsdPtr->lineCLPtr) {
./tclObj.c:1405: hPtr = Tcl_FindHashEntry(tsdPtr->lineCLPtr, objPtr);
./tclObj.c:1495: if (tsdPtr->lineCLPtr) {
./tclObj.c:1496: hPtr = Tcl_FindHashEntry(tsdPtr->lineCLPtr, objPtr);
These are a number of `TclContinuations...` functions which
encapsulate the access and are called by other parts of the core.
> I'm seeing this cost associated with maintaining the line numbers
> consistently being a hot spot in most of my tests (hot spot meaning,
> it's one of the leaf nodes in the performance tree that's taking the
> most time, i.e. one of the first things you would try to fix).
> From how I interpreted your response, it sounds like this
> functionality (maintaining line numbers) is expensive and confusing,
> but probably not an easy thing to disable/remove.
> Is that correct? Would you have any other suggestions/pointers for
> me to look at?
Thinking about things, with the freeing of the information in
TclFreeObj disabled I have two main thoughts
(1) Freeing the CL data of a Tcl_Obj is mostly defered to the end of
the thread, when the thread exit handler
`TclThreadFinalizeContLines` gets invoked.
So, better perf, price is a larger heap.
(2) When a Tcl_Obj is released and its memory is reused, and the new
Tcl_Obj also needs lineCLPtr data.
Then the freeing of the old data happens in
`TclContinuationsEnter` as the system enters the information for
the new Tcl_Obj.
See the comments in that piece of code. This code replaces the
existing data with anything new, releasing the old allocation. And
while this was originally done for the case of shared literals as
the comments exhaustively explain; this looks to be the right
thing to do for a recycled Tcl_Obj* as well.
When you have the code in TclFreeObj disabled, does the core still
pass the test suite ? If you run it udner valgrind, and -DPURIFY, does
it leak memory it was not before you disabled the code ?
Both tests passing and nothing new leaking would be indicators that
the change, the disabling, could be benign.
For a longer-term solution Don's TIP #445 is a start of hiding the
Tcl_Obj details behind a set of functions. The moment that is
completed, i.e. Tcl_Obj not public anymore, then the data currently
held in these adjunct hash tables can move into new fields of the
Tcl_Obj structure, for much faster direct access.
https://core.tcl-lang.org/tips/doc/trunk/tip/445.md
> thanks.
> On Friday, September 7, 2018 at 12:39:06 AM UTC-7, Donal K. Fellows wrote:
>> On 06/09/2018 04:04, Trey Jackson wrote:
>> > I don't have threaded code. These tests are not constantly
>> > sourcing files - so I don't really understand why that line
>> > continuation pointers would be of much use. A solid third to
>> > half of the 6% runtime is actually spent deleting Tcl_Obj which
>> > are dynamically created commands with no continuations at all.
>> 2. For tracking the provenance of strings derived (by backslash
>> substitution) from script literals. This lets you get accurate line
>> numbers of code out of [info frame], but the machinery for doing
>> this is expensive (and quite confusing for most Tcl core
>> developers). This is because we don't store a record of whether we
>> care about this in the values where we care, and that's because the
>> Tcl_Obj structure is a major memory stress point.
>> BTW, neither use actually creates in CreateHashEntry; that's also the
>> lookup function (because :IMPLEMENTATION-DETAIL: the OUT parameter to
>> indicate whether the value was allocated is NULL[*]).
>>
>> Donal.
>> [* IMPLEMENTATION-DETAILs should not be relied on by user code.
>> Especially this one. ]
>> --
>> Donal Fellows — Tcl user, Tcl maintainer, TIP editor.
>
--
See you,
Andreas Kupries <
akup...@shaw.ca>
<
http://core.tcl.tk/akupries/>
Developer @ SUSE (MicroFocus Canada LLC)
<
andreas...@suse.com>
EuroTcl 2018, Jul 7-8, Munich/DE,
http://eurotcl.eu/
Tcl'2018, Oct 15-19, Houston, TX, USA.
https://www.tcl.tk/community/tcl2018/
-------------------------------------------------------------------------------