Tcl Source Code

The following piece of code doesn't work as expected
due to the empty proc body optimization that went into
Tcl 8.4.

proc bar args {}
proc main {} {
    proc bar args {puts "this is bar"}
    bar
}
main

While this example is artificial, I stumbled over this
bug in a situation where a (formerly empty) proc got
redefined through by
 namespace import -force
and invoked immediately afterwards, all within the same
arm of a switch statement. So it took me some time (and
the help of fellow Tcl'ers on the chat) to find out
what's going on.

Logged In: YES 
user_id=148712

The fix has been corrected; it was wrong whenever the
newly-interpreted script returns TCL_BREAK or TCL_CONTINUE.

Logged In: YES 
user_id=148712

Closed again - performance bug 926164 now open.

Logged In: YES 
user_id=72656

IMO this patch should be reversed if miguel's slowdown
numbers are accurate.  This was a known issue, and is a
pathological case that is acceptable when documented.  It's
not acceptable to take up to a 10% slowdown for the core
just to support this.

Logged In: YES 
user_id=148712

Patch committed; we'll improve on it during alpha if
necessary. New bug item for the missing async checks during
empty loops.

Logged In: YES 
user_id=80530

I'm all in favor of making
further improvements, but
how about committing the
fix we have to the HEAD?

Logged In: YES 
user_id=79902

We need to work out what duties the ISC serves and optimise
it for that.  The more it does, the less it can be optimised.

Logged In: YES 
user_id=148712

(Topic now changed to discussion of the patch ... OT for the
bug ticket? Should I rather open a patch ticket?)
Further possible usage for ISC (assuming no redundancy
elimination): enabling of command tracing in bcc'ed code.

Logged In: YES 
user_id=148712

The minimisation of INST_START_CMDs (ISC) looks to me like
(yet another) job for a post-compile optimiser. In general,
if there are two ISCs such that:

  (a) there is no intervening jump or jump target (including
the second ISC)
  (b) there is no intervening invocation or variable access
(in TEBC terms: instructions that do not DECACHE_STACK_INFO)

then the second ISC can be effectively eliminated.

Logged In: YES 
user_id=79902

Hmm.  If the target was the next instruction, that'd be OK.
 However, we'd also need to be careful to only do this in
the case when the body was empty (or at least empty of
bytecodes).  In the non-empty case, the body's own startcmds
would be better.

Without this particular niggle, I think it would be fairly
easy to add the extra information to the compileenv to help
generate only minimal numbers of startcmds, which would
probably help with the speed.

Logged In: YES 
user_id=148712

I was actually thinking of compiling an INST_START_CMD into
every empty loop body. Any downside to that?

Logged In: YES 
user_id=79902

An idea that occurred to me is that we should not issue this
instruction multiple times in a row (currently possible when
compiling a command whose argument is itself a compiled
command) since the first instance will more than adequately
protect all subsequent ones.

On the other issue: Perhaps we need a counter to do the
asynch stuff that is reset when a startcmd instruction is
processed?  We could make the counter have a relatively long
period, of course, so only very evil BC loops would have any
danger of hitting it.

Logged In: YES 
user_id=148712

Note that this patch introduces a new bug that requires
fixingbefore applying:
   while 1 {}
does not check for asyncs.

File Added - 81839: tclbench

File Deleted - 81782: 



File Added - 81838: 729692.patch

Logged In: YES 
user_id=148712

Slightly modified patch and tclbench results attached.
The patch produces a small overall slowdown in the whole
benchmark suite. Bytecode intensive tests are noticeably
slowed down: 5-10% (loops, base64, klist, list iterate, ...)
with two bad ~15-20% cases (sha1, heapsort).

Logged In: YES 
user_id=79902

It all looks like a good patch to me taking a good route to
solve the problem, but I can't test ATM because of the other
things I've got cooking in various sandboxes; I assume that
the updated test suite works?

File Deleted - 81711: 



File Added - 81782: 729692.patch

Logged In: YES 
user_id=148712

Fixed patch to work correctly with precompiled scripts - the
new instruction is effectively a noop for them.

File Added - 81711: 729692.patch

Logged In: YES 
user_id=148712

Enclosed a tentative patch for review; more test cases
needed. Note that this also fixes [Bug 495830].

Logged In: YES 
user_id=148712

Idea - but may be costly in runtime. Define INST_START_CMD
and INST_END_CMD. Let the bc engine have two states:
  - normal N: execute bytecodes
  - exceptional E: find next command from the source,
interpret it

State N:
   INST_START_CMD: check epoch; if ok do nothing, else
switch state to E and rexec INST_START_CMD
   INST_END_CMD: a noop

State E:
    INST_START_CMD: find in the sources the command
corresponding to the next command; interpret it and push its
result on the stack; jump to the instruction after the
corresponding INST_END_CMD
    INST_END_CMD: a noop

It can possibly be made marginally more efficient (in the
sense of it being less costly in state E). But I do think
this approach could work. Feedback?

Logged In: YES 
user_id=79902

I think we're not talking 'impossible' but rather
'exceptionally difficult' and possibly even 'impossible with
the current bytecode engine'.

The nub of the problem is what happens if you redefine
[while] inside a [while] loop and then try to call that
redefined [while] while still executing that loop.  You have
to be able to invalidate sections of bytecode and replace
them with bytecodes of potentially different length without
invalidating the outer bytecodes which are still in use
(change those and you're also altering the semantics of Tcl,
and it's a non-trivial change at best.)

Logged In: YES 
user_id=148712

This is very similar to bug 495830, at least in its causes.
The general solution could well involve defining a new
bytecode for either 'command start' or 'command end' that
checks for (interp validity, epochs,  ...) and switches to
direct string evaluation in exceptional cases.

Logged In: YES 
user_id=80530


OK, this may go nowhere, but here's a thought.

We have epoch counters that tell us whether
we need to recomile a whole unit of bytecode
before executing it.

Some bytecode execution can increment the
epoch counter.   Can't we check for an increment
after we execute those bytecodes, and if we
see one, abandon bytecode execution and
switch over to direct string evaluation?  (leaving
any recompile for the next evaluation of the
whole bytecode unit)

Is this merely difficult, or fundamentally
impossible?

Logged In: YES 
user_id=148712

Not much that we can do about that? This is really a
fundamental bug in the bytecode model: once a bytecode is
running, it can't be modified.
In your example, changes to bytecompiled commands called
from within main's body will not affect main while it is
running. Another example is
   proc tst {} {
       rename set _set
       proc set args {puts GOTCHA}
       set x 1
       rename set {}
       rename _set set
   }
Running tst the new definition of 'set' will be ignored,as
it was bytecompiled.