Menu
▾
▴
#643 new macro TclFreeIfRefCountZero
In various places, objects with refCount zero are not
cleaned up in the error-case.
See branch "novem-freeifrefcountzero" for a start,
there are probably many more places where this
macro is useful. Meant for Tcl 9. Ongoing.
Inspired by the discussion regarding bug 3598580
with Alexandre Ferrieux.
Interesting. But while we're at it, what about frankly switching to "NewObj()_>refCount==1" for novem ?
I admit this will seriously threaten ABI compatibility. But is this still a concern for novem ?
>"NewObj()_>refCount==1" for novem ?
No, I don't think that's a good idea. (but feel free to prove me wrong in a separate branch)
The objects-start-with-refcount-1 approach was tried during the 8.0 alphas, and we switched to the current system as it greatly simplified most basic uses. (OK, it's in pre-history as it is from before the switch to CVS, but I wrote code against 8.0a1 as I desperately needed the performance boost, and I *remember* what it was like. You had to have a lot of temporary variables where now you can just feed objects directly into APIs like Tcl_ListObjAppendObj.)
Yes, I've been told about this already (and probably by you Donal ;).
Would it be possible to be more precise and factual ?
Please give, in this ticket's comment thread, concrete examples of the complex/slow idioms that it would/did entail.
Consider:
Tcl_SetObjResult(interp, Tcl_NewIntObj(123));
That would have to become:
Tcl_Obj *resultObj = Tcl_NewIntObj(123);
Tcl_SetObjResult(interp, resultObj);
Tcl_DecrRefCount(resultObj);
There are a *lot* of other "simple" cases like that (e.g., appending an element to a known list) and the idiom is very common in Tcl itself. (For example, it is there in all of the error handling.) Dictionary construction is even worse.
The cases that get slightly better are those where failures are possible in the "consuming" operation (such as Tcl_SetVar2Ex) but they just gain the ability to omit the Tcl_IncrRefCount from the explicit pattern; they still have all the complexity of dealing with the gory details.
It also breaks code a lot. (This is from memory/experience; I wrote an OO extension — it was fast but hard to use — against 8.0a1 and maintained it through to 8.0.3 or so. The pain from switching over was high, but the code shortened quite a lot.) Code written for one pattern cannot be converted to the other by macro. Yes, we've agreed to look at breaking things where necessary, but this is a lot of breakage for very questionable gain.
Alas, I can't provide factual backup for my assertions on complexity from the time; I didn't keep the code and the app it was part of was under a bizarre license anyway. The best thing to come out of it was a research line that lead to the canonical-list-invoke performance enhancements that went into Tcl sometime in 8.3 or 8.4.
Oh, and pointing to the code is awkward as it isn't in fossil; you'll have to grab the tar archives off of SF (I think that's where we keep those files).
(different "the code" in the two paragraphs; first is my code, second is Tcl source code)
> Tcl_SetObjResult(interp, Tcl_NewIntObj(123));
> .... would have to become:
> Tcl_Obj *resultObj = Tcl_NewIntObj(123);
> Tcl_SetObjResult(interp, resultObj);
> Tcl_DecrRefCount(resultObj);
Unless, of course, the contract of SetObjResult is altered to *not* incrRefCount !
Obviously, after a decade of existence, the create-at-zero principle has pervaded all APIs, and I am not minimizing the cost of a switch. But a fair evaluation must take this "contamination" into account and allow for each solution to assume cooperation from the rest ;)
> Code written for one pattern cannot be converted to the other
> by macro. Yes, we've agreed to look at breaking things where
> necessary, but this is a lot of breakage for very questionable gain.
*This* is the winning argument !
Thanks for recording it here; now we can proceed with Jan's TclFreeIfRefCountZero macro, which respects the current semantics but allows for a cleaner refcount/lifecycle rule for objects: now they should only exceptionally die at -1.
> Code written for one pattern cannot be converted to the other
> by macro
Let's record here why not. Dividing the API in "comsumers"
(like Tcl_SetObjResult, Tcl_ListObjAppendObj,
Tcl_SetVar2Ex) and "producers" (like Tcl_NewIntObj,
Tcl_GetVar2Ex, Tcl_GetObjResult), let's assume we
change the contract: "producers" will either return
a new object with refcount 1, either increment the
refcount of an already existing object and return that.
"consumers" no longer increment the refcount from
the object they receive. That would pretty well
keep existing extension code unchanged... Is it?
One example
Tcl_Obj *resultObj = Tcl_NewIntObj(123);
Tcl_IncrRefCount(resultObj);
printf("%s", Tcl_GetString(resultObj));
Tcl_DecrRefCount(resultObj);
This code works fine in Tcl8, but would lead
to a memory leak in Tcl9. Remove the
Tcl_IncrRefCount, and it will work in both.
Tcl_Obj *resultObj = Tcl_NewIntObj(123);
Tcl_SetVar2Ex(interp, ......, resultObj);
Tcl_SetObjResult(interp, resultObj);
This works in Tcl8, but with a changed contract
an additional Tcl_IncrRefCount should be done.
It's a pretty common pattern: returning the same
object that is operated on. Or, maybe, a
TCL_NEWREF flag could be added
to Tcl_SetVar2Ex, which makes it do the increment
as before. Then - at least - code can be written with
is usable in both Tcl8 and Tcl9. But where is the gain?
However - still - the gain is questionable here, I agree
with dkf on that. I could imagine that Tcl one day is
rewritten to let Tcl_Obj's be shared among threads, then
having a refcount of 0 is dangerous, any thread could
clean it up at will, that could be a good reason for
changing the contract as suggested.
Or maybe when Tcl switches to use garbage collection in
stead of refcounting. But for Tcl9 I don't see that happening,
because it has a lot of other dangerous implications. And
when it happens, the contract can be changed along with it.