Menu
▾
▴
Re: [TCLCORE] CFV warning: TIP #717: New function: Tcl_AttemptCreateHashEntry()
|
From: Ashok N. <apn...@ya...> - 2025-05-05 17:54:12
|
TIP 717 has two proposals that are really independent
*
Tcl_AttemptCreateHashEntry is new API
*
Allowing isNew to be NULL is an enhancement to the existing Tcl_CreateHashEntry
Both look reasonable to me.
I have questions about the implementation.
I do not understand why the implementation does not simply define a new stub entry for Tcl_AttemptCreateHashEntry instead of introducing all that macro bloat.
Consider the compiled code from a single call to Tcl_CreateHashEntry from trunk versus the TIP717 implementation.
Trunk:
240: 48 89 54 24 10 mov %rdx,0x10(%rsp)
245: 48 89 4c 24 08 mov %rcx,0x8(%rsp)
24a: 48 83 ec 28 sub $0x28,%rsp
24e: 4c 8b 44 24 38 mov 0x38(%rsp),%r8
253: 48 8d 15 00 00 00 00 lea 0x0(%rip),%rdx # 25a <CreateHash+0x1a>
25a: 48 8b 4c 24 30 mov 0x30(%rsp),%rcx
25f: 48 8b 44 24 30 mov 0x30(%rsp),%rax
264: ff 50 58 call *0x58(%rax)
267: 48 83 c4 28 add $0x28,%rsp
26b: c3 ret
The same call with TIP 717:
280: 48 89 54 24 10 mov %rdx,0x10(%rsp)
285: 48 89 4c 24 08 mov %rcx,0x8(%rsp)
28a: 48 83 ec 38 sub $0x38,%rsp
28e: 48 83 7c 24 48 ff cmpq $0xffffffffffffffff,0x48(%rsp)
294: 74 2f je 2c5 <CreateHash+0x45>
296: 4c 8b 44 24 48 mov 0x48(%rsp),%r8
29b: 48 8d 15 00 00 00 00 lea 0x0(%rip),%rdx # 2a2 <CreateHash+0x22>
2a2: 48 8b 4c 24 40 mov 0x40(%rsp),%rcx
2a7: 48 8b 44 24 40 mov 0x40(%rsp),%rax
2ac: ff 50 58 call *0x58(%rax)
2af: 48 8d 15 00 00 00 00 lea 0x0(%rip),%rdx # 2b6 <CreateHash+0x36>
2b6: 48 8b c8 mov %rax,%rcx
2b9: e8 00 00 00 00 call 2be <CreateHash+0x3e>
2be: 48 89 44 24 20 mov %rax,0x20(%rsp)
2c3: eb 1e jmp 2e3 <CreateHash+0x63>
2c5: 4c 8b 44 24 48 mov 0x48(%rsp),%r8
2ca: 48 8d 15 00 00 00 00 lea 0x0(%rip),%rdx # 2d1 <CreateHash+0x51>
2d1: 48 8b 4c 24 40 mov 0x40(%rsp),%rcx
2d6: 48 8b 44 24 40 mov 0x40(%rsp),%rax
2db: ff 50 58 call *0x58(%rax)
2de: 48 89 44 24 20 mov %rax,0x20(%rsp)
2e3: 48 83 c4 38 add $0x38,%rsp
2e7: c3 ret
That's something like 4x code bloat (ignoring the function enter/exit boilerplate above) at every call site and probably a performance cost associated with the additional jumps. And that does not even include the additional bloat not shown above from the TclDbPanicIfNull call being added to every file where Tcl_CreateHashEntry is called (luckily the compiler ignores the inline directive else it would be even more code bloat).
So the question is, why the macro wrapper overloading Tcl_CreateHashEntry with the (int *)-1 argument instead of a simple additional stub entry?
I do not like this proliferation of macro use. Here, and in other cases, it makes the compiler generate code for runtime decisions for conditions that are known at compile time while losing some level of type safety.
I also do not understand the macro overloading of Tcl_FindHashEntry -> Tcl_CreateHashEntry. What is the new need in Tcl 9.1 that made this necessary when it was not present in 9.0?
In passing, I also believe the current implementation of the macro is broken but have not created a test case to prove it. That would however be presumably fixable so the real issue is why the macros instead of a stub entry?
/Ashok
________________________________
From: Jan Nijtmans
Sent: Thursday, May 1, 2025 2:52 PM
To: Tcl Core List
Subject: [TCLCORE] CFV warning: TIP #717: New function: Tcl_AttemptCreateHashEntry()
This is a CFV warning for TIP #717. for Tcl 9.1+:
New function: Tcl_AttemptCreateHashEntry()
<https://core.tcl-lang.org/tips/doc/trunk/tip/717.md>
If you think this is a bad idea, speak up now. If not,
I'll start the vote in a few days.
Regards,
Jan Nijtmans
_______________________________________________
Tcl-Core mailing list
Tcl...@li...
https://lists.sourceforge.net/lists/listinfo/tcl-core
S
|
