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 |