[Tcl-bugs] [ tcl-Bugs-729692 ] redef of bcc'ed commands inoperant in current bcode

[SourceForge.net <no...@so...>]

Bugs item #729692, was opened at 2003-04-29 13:27
Message generated for change (Comment added) made by msofer
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110894&aid=729692&group_id=10894

Category: 46. Bytecode Compiler
Group: development: 8.5a2
Status: Open
Resolution: None
Priority: 7
Submitted By: Reinhard Max (rmax)
Assigned to: miguel sofer (msofer)
Summary: redef of bcc'ed commands inoperant in current bcode

Initial Comment:
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.


----------------------------------------------------------------------

>Comment By: miguel sofer (msofer)
Date: 2004-03-30 08:20

Message:
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.

----------------------------------------------------------------------

Comment By: Donal K. Fellows (dkf)
Date: 2004-03-30 06:43

Message:
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.

----------------------------------------------------------------------

Comment By: miguel sofer (msofer)
Date: 2004-03-30 06:24

Message:
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?

----------------------------------------------------------------------

Comment By: Donal K. Fellows (dkf)
Date: 2004-03-30 05:08

Message:
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.

----------------------------------------------------------------------

Comment By: miguel sofer (msofer)
Date: 2004-03-29 21:56

Message:
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.

----------------------------------------------------------------------

Comment By: miguel sofer (msofer)
Date: 2004-03-29 21:41

Message:
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).

----------------------------------------------------------------------

Comment By: Donal K. Fellows (dkf)
Date: 2004-03-29 17:45

Message:
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?

----------------------------------------------------------------------

Comment By: miguel sofer (msofer)
Date: 2004-03-29 15:10

Message:
Logged In: YES 
user_id=148712

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

----------------------------------------------------------------------

Comment By: miguel sofer (msofer)
Date: 2004-03-29 00:22

Message:
Logged In: YES 
user_id=148712

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

----------------------------------------------------------------------

Comment By: miguel sofer (msofer)
Date: 2004-03-10 11:02

Message:
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?

----------------------------------------------------------------------

Comment By: Donal K. Fellows (dkf)
Date: 2004-03-10 08:54

Message:
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.)

----------------------------------------------------------------------

Comment By: miguel sofer (msofer)
Date: 2004-03-09 21:21

Message:
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. 

----------------------------------------------------------------------

Comment By: Don Porter (dgp)
Date: 2004-03-09 19:16

Message:
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?

----------------------------------------------------------------------

Comment By: miguel sofer (msofer)
Date: 2003-11-15 20:24

Message:
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.


----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110894&aid=729692&group_id=10894