Tcl Source Code

View Ticket
Login
Ticket UUID: 1158008
Title: new list internal rep
Type: Patch Version: None
Submitter: msofer Created on: 2005-03-07 01:05:59
Subsystem: 14. List Object Assigned To: msofer
Priority: 5 Medium Severity:
Status: Closed Last Modified: 2011-05-06 01:03:34
Resolution: Accepted Closed By: msofer
    Closed on: 2006-09-29 17:31:39
Description:
Attached a patch against HEAD that changes the internal
reps of objects. The changes are:

(1) The List struct is now growable and contains the
pointers to the list elements at the end. This reduces
the number of mallocs in list creation from 2 to 1.

(2) The List struct is now refcounted. This permits
insuring that the pointers to the list elements survive
a possible shimmering of the object (by increasing the
ref count). This is now use in Tcl_EvalObjEx (pure list
opt) and TclLsetList (so that it calls TclLsetFlat)

(3) All empty lists in a thread share the same List
internal rep. In this manner, an empty list can be
allocated and reserved without any malloc/free calls

The patch passes the testsuite. It has not been tested
for leaks yet. There shouldn't be any ...

CAVEAT: callers of Tcl_ListObjGetElements should *not*
change the elements, even if the Tcl_Obj is unshared
(as its internal rep may be shared). This was IMO
already an abuse of the api, but it is valid in the
unpatched implementation and invalid with the patch.
The docs are silent about this; they do not even
mention that the Tcl_Obj should not be shared for an
edit to be valid. Maybe throw some new CONST into that api?
User Comments: dgp added on 2011-05-06 01:03:34:
Six years later, the question about all the empty list
special case effort arises again.  Please compare
core-8-5-branch and dgp-list-simplify and tell me
what benefits of avoiding List allocation for empty
lists I'll be sorry doing without on the dgp-list-simplify branch.
Thanks.

dkf added on 2005-04-25 17:05:42:
Logged In: YES 
user_id=79902

Assigning to the person who started checking in code since
it seems to have caused someone problems.

jenglish added on 2005-04-25 14:02:53:
Logged In: YES 
user_id=68433

See also: #1189274  and #1178805.

I don't think all the special-case checks for empty lists
and all the new bugs (including the ones that haven't been
discovered yet) are worth trying to save a malloc() here and
there.  Please reconsider the empty list special case.

dkf added on 2005-04-05 22:41:12:
Logged In: YES 
user_id=79902

Random thought: I wonder whether it would be possible to
allocate List instances from the same pool as Tcl_Objs?

dkf added on 2005-04-05 21:36:32:
Logged In: YES 
user_id=79902

My #1 issue is that var length structures are a mess. They
require all sorts of nasty things to work with them, and
must be tested rigorously on lots of architectures because
of alignment issues. :^(

If we pool List instances, the likely number of allocations
per list creation is still just one and we don't have to do
address munging to make it all "just work".

msofer added on 2005-04-02 08:56:06:

File Added - 128163: listPatch2

Logged In: YES 
user_id=148712

New patch attached (committed to HEAD). Changes are:

* better resolution of the "empty list" issue: an empty
Tcl_Obj is always returned in that case, calling
Tcl_ListObjGetElements on it returns length==0 and
objv==NULL. All list ops know how to detect and operate with
empty lists.

* warning in the docs about not mucking with the contents of
the array returned by Tcl_ListObjGetElements is strengthened

This dissolves the TSD issue, as there is no sharing of
empty List structs.

Leaving "Pending" and assigning to donal for clarification
of the issues with the var-length structs.

dkf added on 2005-04-01 19:13:45:
Logged In: YES 
user_id=79902

I'm not convinced yet that combining the array and the
structure is a good idea. Perhaps pooling the List instances
would be a better approach, but that's impossible with
var-length structures.

I also wonder whether sharing the List just within an Interp
would be easier?

msofer added on 2005-03-09 02:11:12:
Logged In: YES 
user_id=148712

Joe English remarks that TSD access may be expensive, so
that the proposed sharing of empty lists may not be
worthwhile. One solution is to alloc them on demand (code
for that is #if-zeroed out in the patch), another one is to
define a struct for thread-specific data, store a pointer in
a tsd and another pointer (at creation time) in each interp
in that thread.

msofer added on 2005-03-07 08:05:59:

File Added - 124431: listPatch

Attachments: