Re: [Tcl-bugs] [Tcl] (jan.nijtmans) com (sebres-8-6-clock-speedup-cr1): Fix LookupLastTransition() for behavior when tick TclEvalObjEx change, eliminate two inline macro's which are only used once. Eliminate many unnecessary MODULE_SCOPE declarations (duplicated with header files)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Tcl-bugs] [Tcl] (jan.nijtmans) com (sebres-8-6-clock-speedup-cr1): Fix LookupLastTransition() for behavior when tick TclEvalObjEx change, eliminate two inline macro's which are only used once. Eliminate many unnecessary MODULE_SCOPE declarations (duplicated with header files)

Jan Nijtmans-2
2017-06-07 14:05 GMT+02:00 Dipl. Ing. Sergey G. Brester
<[hidden email]>:

> Hi, Jan
>
> Tanks for review.
>
> I do not understand 2 things in [0c978a034b]:
>
> LookupLastTransition() (l. 2370) what was disturbing on "goto done" here?
> This obscure case (first row doesn't begin at MIN_WIDE_INT) used al most
> never (on good TZ)
> and your change makes the routine as well as byte-code larger.

The difference is the previously, this function did:
    return rowv[0];
and with your change, in this specific situation:
    return rowv[1];

I don't know exactly in what situation it happens, but it's a difference
which is not augmented with a test-case or documentation change.
Since your work should be a performance-only work, any unrelated
code change is suspicious, so I'm just trying to do you a favor.   ;-)

>
> TclPreserveByteCode / TclReleaseByteCode  (eliminate two inline macro's
> (which are only used once)) -
>
> this was trunk compatible (back ported from there), now you'll get your
> conflicts there.
> Note in trunk there are used not once (and both are public in tclInt.h).

Since you moved those macro's to tclBasic.h, there already was a
conflict, again .... I think I did you a favor.

> You are the boss, but I'll suggest to revert both above mentioned things. :)

Actually ....  You are the boss (it's your branch!). But since I expect that
more people than only me will review this branch, any change which is
unrelated to the intent of your work will make the review process more
complicated. So, less likely to pass for 8.6.8 (eventually)

For Tcl 8.7, I would gladly accept such changes (preferably accompanied
with a test-case), but for a patch release - which should contain bug-fixes
and performance improvements only - more care should be taken. At
least that's my point of view.

Regards,
         Jan Nijtmans

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Loading...