On Tue, Dec 19, 2023 at 08:46:41PM +0800, Qian Yun wrote:
>
>
> On 12/19/23 20:19, Waldek Hebisch wrote:
> > On Tue, Dec 19, 2023 at 05:58:37PM +0800, Qian Yun wrote:
> > >
> > >
> > > On 12/19/23 08:01, Waldek Hebisch wrote:
> > > >
> > > > Well, original had:
> > > >
> > > > while savedTimerStack ^= $timedNameStack repeat
> > > > stopTimingProcess peekTimedName()
> > > >
> > > > 'stopTimingProcess' calls 'updateTimedName' which was/is needed
> > > > to update counters. IIUC without this statistics will be wrong
> > > > (time will be "charged" to different use).
> > >
> > > 'stopTimingProcess' will run only when exceptions are thrown and
> > > catched. In this case, stats are already messed up.
> >
> > I do not think so. More preciesely, 'startTimingProcess' adds
> > position to '$timedNameStack' and causes timing to be charged
> > to given name up to point of another 'startTimingProcess'
> > or 'stopTimingProcess'. When 'THROW' happens time is still
> > charged to code responible for 'THROW' which is more or less
> > resonable. Normal return would call 'stopTimingProcess', the
> > loop simulates this.
>
> I can agree on this part.
>
> > > > > Waldek, what's your opinion on fixing this bug?
> > > >
> > > > AFAICS loop here and in interpretTopLevel where right. I am not
> > > > sure if the loop is really necessary, but we need to do proper
> > > > accounting and the loop is "obviously correct" way of doing this.
> > > > Another approach may easily get broken by changes.
> > > >
> > > > IMO proper fix for this bug involves making sure that we always
> > > > have valid '$timedNameStack' and that it is in sync with other
> > > > variables. One way to ensure this is doing "push" on init,
> > > > instead of assignment.
> > > >
> > >
> > > OK, to recap on this problem:
> > >
> > > We need to make sure '$timedNameStack' is valid.
> > >
> > > '$timedNameStack' can be messed up in two cases:
> > >
> > > 1. recursion
> > > 2. thrown and catch exceptions
> > >
> > > For 1, correct way is to use dynamic bindings.
> >
> > Dynamic bindings would be good if the loop was not present. We
> > need to make sure that variables are still valid when we execute
> > the loop.
> >
> > I think that simplest solution is to avoid _assignment to
> > '$timedNameStack'. Instead initialization should push, so
> > that when there is timing in progress poping will get old value.
>
> If you want to use "push", then what happens to not catched errors,
> for example "1/0"?
Well, eventually something should catch error. And the place catching
it should do the unwinding loop. We need to look what is simpler:
unwinding loop at all relevant catchers (many only catches where
timing context changes are relevant) or putting 'stopTimedName'
inside UNWIND-PROTECT cleanup.
> Then '$timedNameStack' will keep growing.
Well, if cleanup is done correctly than no.
> So I think use assignment to initialize this dynamic variable
> is the correct approach.
Assignment has trouble with accounting
--
Waldek Hebisch
--
You received this message because you are subscribed to the Google Groups
"FriCAS - computer algebra system" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/fricas-devel/ZYHMDY0yGGYV7rWm%40fricas.org.
