Ticket UUID: | 3072080 | |||
Title: | a saner NRE | |||
Type: | Patch | Version: | None | |
Submitter: | msofer | Created on: | 2010-09-20 18:41:24 | |
Subsystem: | 60. NRE and coroutines | Assigned To: | msofer | |
Priority: | 5 Medium | Severity: | ||
Status: | Closed | Last Modified: | 2010-09-28 03:03:27 | |
Resolution: | Accepted | Closed By: | msofer | |
Closed on: | 2010-09-27 20:03:27 | |||
Description: |
Attached a patch that removes all NRE duties from TEBC: it is a relatively big redesign of the NRE main loop. The original NRE implementation has TclNRRunCallbacks (TNRRC) and TEBC work in tandem, working hard to reuse TEBC instances. The design is at the root of a very complicated logic flow within TEBC and elsewhere. This patch removes the special NRE status from TEBC. The main advantage is that the logic is much simpler (both within TEBC and elsewhere), leading to enhanced maintainability. I would appreciate more thorough testing (and perf impact estimates?) before committing this patch. | |||
User Comments: |
msofer added on 2010-09-28 03:03:27:
allow_comments - 1 Committed msofer added on 2010-09-28 01:16:06: File Added - 388051: inv.diff msofer added on 2010-09-28 01:15:17: updated patch - ready to commit as soon as I find out how to handle the 1-line change to itcl ferrieux added on 2010-09-23 13:45:25: File Added - 387605: sn2_summary.txt ferrieux added on 2010-09-23 13:44:56: File Added - 387604: decache_summary.txt ferrieux added on 2010-09-23 13:44:07: Moving target, aiming again ;-) Attaching new timings: - decache_summary.txt just compares before and after the DECACHE_STACK_INFO commits. I know they were necessary, but IMO it's interesting to keep an eye on the needle when doing brain surgery. Result: a maximum of 6% slowdown, not significant given the other deviations with/out the spacer. - sn2_summary.txt compares current HEAD with inv2.diff. Result: still <=12%... msofer added on 2010-09-23 04:17:20: File Added - 387566: inv2.diff msofer added on 2010-09-23 04:16:51: File Deleted - 387539: msofer added on 2010-09-22 23:02:50: File Added - 387539: inv2.diff msofer added on 2010-09-22 23:01:49: New patch; please apply to HEAD including the 2010-09-22 fixes msofer added on 2010-09-22 18:45:40: Mem corruption: some obj's typePtr points to some (other?) obj's string rep: see valgrind's complaint below. Does not at first look like a refCount issue: this is a -DPURIFY build, so that individual objs (and TclSmallAlloc) are all done by calls to ckalloc. ==3585== Invalid read of size 4 ==3585== at 0x807B28B: TclFreeObj (tclObj.c:1413) ==3585== by 0x815C50E: TclResumeByteCode (tclExecute.c:6324) ==3585== by 0x80DCC7B: TclNRRunCallbacks (tclBasic.c:4312) ==3585== by 0x80DEE3F: TclEvalObjEx (tclBasic.c:5887) ==3585== by 0x80DEDCE: Tcl_EvalObjEx (tclBasic.c:5868) ==3585== by 0x816871B: Tcl_RecordAndEvalObj (tclHistory.c:192) ==3585== by 0x80736E6: Tcl_Main (tclMain.c:471) ==3585== by 0x80564C4: main (tclAppInit.c:85) ==3585== Address 0x4317cbf is 127 bytes inside a block of size 196 free'd ==3585== at 0x4024B3A: free (vg_replace_malloc.c:366) ==3585== by 0x818E817: TclpFree (tclAlloc.c:730) ==3585== by 0x80E7B34: Tcl_Free (tclCkalloc.c:1207) ==3585== by 0x807B272: TclFreeObj (tclObj.c:1410) ==3585== by 0x815C50E: TclResumeByteCode (tclExecute.c:6324) ==3585== by 0x80DCC7B: TclNRRunCallbacks (tclBasic.c:4312) ==3585== by 0x80DEE3F: TclEvalObjEx (tclBasic.c:5887) ==3585== by 0x80DEDCE: Tcl_EvalObjEx (tclBasic.c:5868) ==3585== by 0x816871B: Tcl_RecordAndEvalObj (tclHistory.c:192) ==3585== by 0x80736E6: Tcl_Main (tclMain.c:471) ==3585== by 0x80564C4: main (tclAppInit.c:85) msofer added on 2010-09-22 18:18:15: Note that this patch also fixes Bug #3072640 ferrieux added on 2010-09-21 04:19:28: Also, var.test panics on my system (unix threaded, nondebug): Test file error: TclStackFree: incorrect freePtr (0x8421f48 != 0x8421f3f). Call out of sequence? ferrieux added on 2010-09-21 03:44:25: File Added - 387302: sn_summary.txt ferrieux added on 2010-09-21 03:40:17: Attaching perf measurements with the "10-bench + spacer" technique. The first four columns are normalized times. First line headers give the names: - tcl86 == vanilla HEAD - tcl86foo == HEAD + 16-byte foo spacer at beginning of regcomp.o - tcl86sn == HEAD + Saner Nre patch - tcl86snfoo == both File sorted on column 3, which is tcl86sn time. Bottom line: hard to summarize... range is roughly +-12%, though near both extremes the foo-spacer wipes the effect. Tough... msofer added on 2010-09-21 01:41:24: File Added - 387291: inv.diff |
Attachments:
- inv.diff [download] added by msofer on 2010-09-28 01:16:06. [details]
- sn2_summary.txt [download] added by ferrieux on 2010-09-23 13:45:25. [details]
- decache_summary.txt [download] added by ferrieux on 2010-09-23 13:44:55. [details]
- inv2.diff [download] added by msofer on 2010-09-23 04:17:20. [details]
- sn_summary.txt [download] added by ferrieux on 2010-09-21 03:44:25. [details]