Intent to work on speedups to clock format and clock scan

[aidanhs]

No description provided.

[aidanhs]

Request for clarification: for clock scan, is the interest in the free-form scanning or scanning of a particular -format? I've found this flightaware benchmark which uses free-form but just want to check that I'm considering the correct code (given that the free-form scan is apparently 'deprecated').

[bovine]

Generally clock scan -format is probably more interesting to focus on.

[sebres]

I've the Intention also, but I've already implemented both (scan and format) it in pure-C (also with bison, similar to tclGetDate.y), however in my own tcl-fork (tclSE-mod - mix from 8.5, current trunk and my own), that are unfortunately not completely compatible with standard tcl-core, so my work would be currently:

• port it to tcl8.6 (or trunk ?)
• compatibility fixes and code review
• port the test cases to tcl.core tests/clock.test (my test-framework is a totally different, but I use it, because it has real code coverage procedure);

As a first I need to know, which code-base would preferred - 8.6 or 8.7?

And about of performance (tcl86 compared with my tclSE-mod, what is self a good piece faster, so I don't know which amount goes thereby to clock), for example:
Both tcl's are threaded, warmed up, 4 x threads working, 4 core i5-2400 @ 3.10 GHz.

Click here to expand

% set d 2016-12-01 01:08:01
# 1.a. default tcl -- :
% time {clock scan $d -format "%Y-%m-%d %H:%M:%S"} 100000
72.76242 microseconds per iteration
% set c 1480550881
# 2.a. default tcl -- :
% time {clock format $c -format "%Y-%m-%d %H:%M:%S"} 100000
13.3468 microseconds per iteration

% set d 2016-12-01 01:08:01
# 1.b. tclSE-mod -- :
% time {clock scan $d -format "%Y-%m-%d %H:%M:%S"} 100000
6.9054 microseconds per iteration
% set c 1480550881
# 2.b. tclSE-mod -- :
% time {clock format $c -format "%Y-%m-%d %H:%M:%S"} 100000
3.33677 microseconds per iteration

Would be that enough for 20K? 😄

Additionally I've enhancement with several new C commands, like Tcl_MakeDate (to fast make a clock wideInt from C time structures, e. g. for the database-libraries, etc.)

But it also not 100% compatible, because I've fixed several bugs, for example see my fix in default tcl.core http://core.tcl.tk/tcl/tktview?name=3475995, that assigned to kennykb already since 4 years.

Click here to expand

# tcl (without my abovementioned fix) -- :
% clock format [clock scan "2008-02-30"]
Sat Mar 01 00:00:00 CET 2008
% clock format [clock scan "24:62:80"] -format {%T}
23:59:59

# tcl (with my fix) or tclSE-mod -- :
% clock format [clock scan "2008-02-30"]
unable to convert date-time string "2008-02-30": invalid day
    while executing
"clock scan "2008-02-30""
% clock format [clock scan "24:62:80"] -format {%T}
unable to convert date-time string "24:62:80": invalid time
    while executing
"clock scan "24:62:80""

# to call "clock scan" without validating (backwards compatible):
% clock format [clock scan "2008-02-30" -valid 0]
Sat Mar 01 00:00:00 CET 2008
% clock format [clock scan "24:62:80" -valid 0] -format {%T}
23:59:59

[bovine]

It would probably be most natural to get your changes first reviewed and accepted into trunk by the Tcl core team, and then as a followup make a backport to Tcl 8.6 based on what ultimately got accepted.

If you haven't already, ensure that all of the Tcl unit tests concerning clock still pass. Also, we're not stopping you from achieving even more gains! :)

[sebres]

first reviewed and accepted into trunk by the Tcl core team

I belong to tcl-core team already several years, and I know unfortunately good enough how "fast" some enhancements (but sometimes the bugs also) would be accepted there from the whole team :(
Critical bug - yes, all others - well, well, well.
Look for example at the bug 3475995 I meant above.
And I had a 3 branches to it, that was also never merged.

We can wait years... I mean, I do the lot of work to port all of this to trunk, and then wait again 4 years for the team acceptance (and still convince they all, that this is good), to take only then the bounty?
I think, my interest to intent shrinks a little bit...

[aidanhs]

I want to point out that it's (relatively) easy to make clock scan an incredible amount faster if you break backwards compatibility. From my analysis, the hard part is doing it without breaking backwards compat - if you can still get a 10x speedup then I'd be pretty impressed. For example, you mention using bison...but even with -format a date parse may be ambiguous, which I think complicates things? I'd be interested to see what you've done in tclSE-mod though.

This isn't intended to downplay the good work you've done @sebres. It's more an observation that clock scan being very forgiving in inputs (even with -format) has significant effects on performance. Indeed, if flightaware are happy with strict date parsing (perhaps in the form of a -strictformat option?) then that's the route to go down for sure (and would be faster even if implemented in tcl).

For my part, I have a strategy which I believe should give a >= 2x speedup with 100% backwards compatibility. However, it's highly complex and I've had a few things move up in my list of priorities so I won't be able to get to it for a month or two - I encourage other interested parties to have a look in the meantime :)

[sebres]

easy to make clock scan an incredible amount faster if you break backwards compatibility.

1. I did not. It is currently not completely backwards compatible because of bug fixing (so breaks also some test cases), but I can revert this changes. And because in my version the date validation parameter -valid is default on. For backwards compatibility I can just set it to off.
2. I cannot say that was easy (and the lot of work was not result of the backwards compatibility, trust me).

However, it's highly complex

Well you should choose at last: it is "easy going" or "it's highly complex" ;)

For example, you mention using bison...but even with -format a date parse may be ambiguous, which I think complicates things?

Well, even the FreeScan (resp. its c-part Oldscan, see tclDate.c) made and described with yacc (bison). And this kind of scan is a lot more complicated as "exact" format to clock conversion... And that's still understatement.
The complexity by exact specified format goes to make the whole thing "marshallable" and "cacheable" (e. g. by formats using dynamic things like locale, etc.).
The rest is trivially.

But I had also more as one branch optimizing clock (one ot two was without bison). I'll see which can be better ported.

[aidanhs]

I cannot say that was easy (and the lot of work was not result of the backwards compatibility, trust me).

Well you should choose at last: it is "easy going" or "it's highly complex" ;)

I only said it was easy if you break backwards compatibility. Since you preserved backwards compatibility, I can't imagine it being easy :)

And this kind of scan is a lot more complicated as "exact" format to clock conversion... And that's still understatement.

Hmm I'm not convinced, the rules of FreeScan seem unambiguous (though opaque) - after a quick look, I can't see any string that would be valid to parse as something else. By contrast, consider 1111 with -format %y%m%d - you can parse this as any one of 2011-01-01, 2001-11-01 or 2001-01-11. Regexps being greedy resolves this ambiguity in the current implementation:

% clock format [clock scan 1111 -format %y%m%d]
Fri Jan 01 00:00:00 GMT 2011

My bison knowledge is weak, but isn't that tricky to express? And the "you can have any number of spaces before a field" also seems to interact poorly here. Of course, I may be totally wrong.

Anyway, I'll wait and see what you have. Even if you decide not to submit I hope you share your changes for others to take a look at.

[sebres]

Regexps being greedy resolves this ambiguity in the current implementation

Such kind of match (no matter greedy or not) for 3 (or even 6) KNOWN actors is trivial in realization without any NFA, etc.

So the default rule by greedy NFA: at least 2 max 4 digits for year 1-2 digits for month and day, if not "matched" step back, so repeat with fewer "greedy" count.

Even without regexp it can be build more faster, because we can quick estimate which counts could be used (with preparing already by parsing of format, if detected that no separator between used). So the formula for counts is: if not "separators len" (0) : y + m + d <= 4 (standing together digits len), what allows to range counts for all actors to 2, 1, 1 before (resp. during) the match-search...

[aidanhs]

Even without regexp it can be build more faster, because we can quick estimate which counts could be used (with preparing already by parsing of format, if detected that no separator between used).

I'm aware of this approach and it being easy in C, it was just meant to be a simple example of ambiguity that I wasn't clear how bison would handle. The complex cases for a C implementation come when you try to allow spaces prefixing fields. I can come up with increasingly difficult examples which are always possible to special case, but I may as well just wait until there's something I can see - I assume tclSE-mod isn't publicly visible right now?

[sebres]

I assume tclSE-mod isn't publicly visible right now?

Currently not planned. But in the context of subject it is not of importance.
I have him mentioned only because I've implemented newer clock commands on its codebase (so incompatible - cherry-pick, conflict, cherry-pick, etc. ;)
And because of the statement about performance - I don't know with which amount he takes a part there, because several function not yet completely rewritten in C and stay coded in tcl (e.g. GetSystemTimeZone, etc.) So my measurement can deviate a little bit.

I work on the clock porting, maybe I'll get something running the next week (not yet promised that all test cases pass then).

[sebres]

I'd be interested to see what you've done in tclSE-mod though.

Because it not belongs to the subject of this issue - here a short description why and how I wrote tclSE...

[kennykb]

Hi, I've heard my name mentioned - and I do see that 3475995 is assigned to me! Because of the title, I'd mentally bucketed that one as, "needs a TIP", and even "not sure this is a good idea."

The reason for "needs a TIP" is that the change as called out is for the date and time validation, and that part is a user-visible API change. This isn't a problem, necessarily - and if you write up a TIP, I'll sponsor.

The reason for "not sure this is a good idea" is that it's actually useful to be able to specify day 0 of a month (the last day of the preceding month) or week 0 of a year (the last week of the preceding year). I see a '-valid' parameter in the discussion here - is the implementation that you attached to 3475995 current?

Because of the subject, and the early discussion, and limited attention (sorry!) I had missed that the change was also intended to address performance. I'd feel most comfortable separating the two concerns, since the performance part requires no public discussion.

Now to some of the details, since there is a little bit of confusion above:

The use of 'regexp' in the parser can be regarded as a red herring - except that it pretty closely describes the syntax that's accepted. The whole trick of using a generated regexp was simply a hack to squeeze as much performance as possible out of the code. In retrospect, it would have been better to start on a C implementation before resorting to that. It's made the code confusing, and served as an obstacle to an eventual C implementation.

The regexps that are generated are of a very limited form: "accept a decimal integer of up to N digits", "accept one of a set of (possibly locale dependent) enumerated strings (or, possibly, a unique prefix of one of them)" - and little else. They could be handled with stylized C code. Having a little table generator to make transition tables for the enumerations would help greatly in parsing things like %Ey
and %Em. Otherwise, the processing is nearly the same sort of job that 'scanf' does.

Ambiguities are always resolved greedily, so "a day of the year is always 1-3 digits" means that as long as digits are seen (up to the third one) they are accepted. I think the only case where any sort of backtracking might be required is the fairly bizarre one of having one enumeration directly follow another. A test case would be 'augusthursday' where 'augus' is a unique prefix of 'august' but consuming the 't' would fail to recognize the weekday. In the case of directly concatenated enumerated strings, ambiguities should resolve by choosing the longest match for the left-hand string that still yields a match for the rest of the pattern. A torture test case could be built using the Roman numeral locale that exists in the test suite: give a string like 'xiiiv' - which should break to xiii-v by that rule.

These matches are simple enough (there's no Kleene closure at all) that I think any given -format could quite readily be reduced directly to a DFA, and the DFA could be cached as an internal representation to the format string. (Along with locale, of course, because that changes the interpretation of some of the format codes.)

I should have a little bit of bandwidth in the coming few weeks to look at [clock] performance - and I'm now monitoring this forum. Feel free to get in touch.

For what it's worth, given the already-complicated history, I'd consider it a conflict of interest to accept any bounty for this item. If participants consider any work I do, moving forward, as a significant contribution toward earning the bounty, whatever share is allocated to my work should go to the Tcl Community Association.

[sebres]

I'm almost ready with back-porting from my tclSE and rewriting to current trunk (with small bug fixes).

• clock scan is 90%-95% ready (free scan completely, format scan missed code review and some clean-ups)
• clock format ca. 50% ready...

Bellow you'll find the first results of performance measurement saved as diff.
As you can see in 2 test cases not the times only are different (but values also) - that was bug fixed from me (and covered in new version also in clock.test).

The test-script for the measurement you'll find sebres/tcl@trunk...sebres:sb/trunk-clock-perf-test#diff-bc64daab7239ff6091b52c773bebd038 .
It expects timerate command, that is also included in tcl-core in this branch.
Each case part runs exactly 0.5 seconds... so the speed (3rd output) is estimated.

Performance measurement test results:

Click here to expand

 % # Scan : date
 % clock scan "25.11.2015" -format "%d.%m.%Y" -base 0 -gmt 1
 Wed Nov 25 01:00:00 CET 2015
-38.2629 µs/#, 13068 #, 26135.0 #/sec
+0.782300 µs/#, 639141 #, 1278282 #/sec
 
 % clock scan "1111" -format "%d%m%y" -base 0 -gmt 1
 Thu Jan 11 01:00:00 CET 2001
-31.4878 µs/#, 15880 #, 31758.3 #/sec
+0.794990 µs/#, 628939 #, 1257878 #/sec
 
 % # FreeScan : relative date
 % clock scan "5 years 18 months 385 days" -base 0 -gmt 1
 Thu Jul 21 02:00:00 CEST 1977
-42.4871 µs/#, 11769 #, 23536.5 #/sec
+1.998985 µs/#, 250127 #, 500254 #/sec
 
 % # FreeScan : relative date with relative weekday
 % clock scan "5 years 18 months 385 days Fri" -base 0 -gmt 1
 Fri Jul 22 02:00:00 CEST 1977
-47.2043 µs/#, 10593 #, 21184.5 #/sec
+2.330546 µs/#, 214542 #, 429084 #/sec
 
 % # FreeScan : relative date with ordinal month
 % clock scan "5 years 18 months 385 days next 1 January" -base 0 -gmt 1
-Fri Jul 21 02:00:00 CEST 1978
-71.3663 µs/#, 7007 #, 14012.2 #/sec
+Sat Jan 21 01:00:00 CET 1978
+2.579274 µs/#, 193853 #, 387706 #/sec
 
 % # FreeScan : relative date with ordinal month and relative weekday
 % clock scan "5 years 18 months 385 days next January Fri" -base 0 -gmt 1
-Sat Jul 22 02:00:00 CEST 1978
-76.3095 µs/#, 6553 #, 13104.5 #/sec
+Fri Jan 27 01:00:00 CET 1978
+2.841803 µs/#, 175945 #, 351889 #/sec
 
 % # FreeScan : ordinal month
 % clock scan "next January" -base 0 -gmt 1
 Fri Jan 01 01:00:00 CET 1971
-42.5941 µs/#, 11739 #, 23477.4 #/sec
+1.360866 µs/#, 367413 #, 734826 #/sec
 
 % # FreeScan : relative week
 % clock scan "next Fri" -base 0 -gmt 1
 Fri Jan 09 01:00:00 CET 1970
-17.9997 µs/#, 27779 #, 55556.3 #/sec
+1.471246 µs/#, 339848 #, 679696 #/sec
 
 % # FreeScan : relative weekday and week offset 
 % clock scan "next January + 2 week" -base 0 -gmt 1
 Fri Jan 15 01:00:00 CET 1971
-69.9115 µs/#, 7152 #, 14303.8 #/sec
+1.816735 µs/#, 275219 #, 550438 #/sec
 
 % # FreeScan : time only with base
 % clock scan "19:18:30" -base 148863600 -gmt 1
 Thu Sep 19 20:18:30 CET 1974
-13.0945 µs/#, 38185 #, 76368.2 #/sec
+1.091822 µs/#, 457950 #, 915900 #/sec
 
 % # FreeScan : time only without base, gmt
 % clock scan "19:18:30" -gmt 1
 Sat Dec 10 20:18:30 CET 2016
-12.9911 µs/#, 38488 #, 76975.5 #/sec
+1.120993 µs/#, 446033 #, 892066 #/sec
 
 % # FreeScan : time only without base, system
 % clock scan "19:18:30"
 Sat Dec 10 19:18:30 CET 2016
-12.4922 µs/#, 40025 #, 80049.8 #/sec
+1.439487 µs/#, 347346 #, 694692 #/sec
 
 % # FreeScan : date, system time zone
 % clock scan "05/08/2016 20:18:30"
 Sun May 08 20:18:30 CEST 2016
-13.3555 µs/#, 37438 #, 74875.4 #/sec
+1.642088 µs/#, 304491 #, 608980 #/sec
 
 % # FreeScan : date, supplied time zone
 % clock scan "05/08/2016 20:18:30" -timezone :CET
 Sun May 08 20:18:30 CEST 2016
-14.0496 µs/#, 35589 #, 71176.4 #/sec
+1.596128 µs/#, 313258 #, 626516 #/sec
 
 % # FreeScan : date, supplied gmt (equivalent -timezone :GMT)
 % clock scan "05/08/2016 20:18:30" -gmt 1
 Sun May 08 22:18:30 CEST 2016
-13.8868 µs/#, 36006 #, 72010.8 #/sec
+1.292126 µs/#, 386960 #, 773918 #/sec
 
 % # FreeScan : date, supplied time zone gmt
 % clock scan "05/08/2016 20:18:30" -timezone :GMT
 Sun May 08 22:18:30 CEST 2016
-13.9869 µs/#, 35748 #, 71495.7 #/sec
+1.285432 µs/#, 388975 #, 777948 #/sec
 
 % # FreeScan : time only, numeric zone in string, base time gmt (exchange zones between gmt / -0500)
 % clock scan "20:18:30 -0500" -base 148863600 -gmt 1
 Fri Sep 20 02:18:30 CET 1974
-17.7084 µs/#, 28236 #, 56470.3 #/sec
+2.634612 µs/#, 189782 #, 379562 #/sec
 
 % # FreeScan : time only, zone in string (exchange zones between system / gmt)
 % clock scan "19:18:30 GMT" -base 148863600
 Fri Sep 20 20:18:30 CET 1974
-17.8210 µs/#, 28058 #, 56113.5 #/sec
+3.049543 µs/#, 163959 #, 327918 #/sec
 
 % # FreeScan : fast switch of zones in cycle - GMT, MST, CET (system) and EST
 % clock scan "19:18:30 MST" -base 148863600 -gmt 1; clock scan "19:18:30 EST" -base 148863600; 
 Sat Sep 21 01:18:30 CET 1974
-35.7111 µs/#, 14002 #, 28002.5 #/sec
+9.217380 µs/#, 54246 #, 108490 #/sec
 
 % # Bad zone
 % catch {clock scan "1 day" -timezone BAD_ZONE -locale en}
 1
-9.413631 µs/#, 53115 #, 106228 #/sec
+7.017994 µs/#, 71246 #, 142490 #/sec
 
 % **STOP**

[sebres]

Short interim status: almost ready, but...

But by running of the final tests, I've unfortunately found large deviations in comparision to the tclSE.

The msgcat module was to blame (in my version it was totally rewritten using my OO-extension, what would be ATM not portable to the current tcl-core).

The problem is - basically I've sped up the clock-engine up to 0.5µs, but by first access of mc in-between in some cases (l10n, entering another locale, etc.) it degraded up to 4.0µs...

I'm working on proper caching of msgcat now (cacheable somewhere in internal representation). Why it should be, you'll see in the example below:

% info patchlevel
- 8.7a0
+ 9.0-SE.18
% ::tcl::clock::EnterLocale de_DE
de_de
% namespace inscope ::tcl::clock { mc MONTHS_FULL }; # short warm up
Januar Februar März April Mai Juni Juli August September Oktober November Dezember {}
% namespace inscope ::tcl::clock { timerate {mc MONTHS_FULL} 500 }
- 3.127795 µs/#, 159857 #, 319714 #/sec
+ 0.106506 µs/#, 4694583 #, 9389166 #/sec

[sebres]

Here you go - sebres/tcl#2

I'm working currently on source code clean-ups: like in-line documentation and minor reviews.
Then I can push it to fossil repository of core.tcl.tk.

Current performance increase:

• clock format - 10-15 times faster;
• clock scan -format - 30-60 times (some previously extremely slow scans up to 100 times faster);
• clock scan (freescan) - 15-20 times;
• clock add - 30-90 times;

If someone needs compiled version to test it, please notify me.

[sebres]

I've made a (threaded) build of newest binaries for Windows. If someone needs it, please see sebres/tcl#2 (comment)

[aidanhs]

A couple of incompatibilities as compared between your windows binaries of tcl running under wine and a freshly built tcl off the core_8_6_6 tag.

core_8_6_6:

% clock format [clock scan "1111 120" -format "%y%m%d %H%M%S"]
Sat Jan 01 01:02:00 GMT 2011
% clock format [clock scan "11 1 120" -format "%y%m%d %H%M%S"]
Mon Jan 01 01:02:00 GMT 2001

Your binaries:

% clock format [clock scan "1111 120" -format "%y%m%d %H%M%S"]
input string does not match supplied format
% clock format [clock scan "11 1 120" -format "%y%m%d %H%M%S"]
input string does not match supplied format

---

@kennykb

These matches are simple enough (there's no Kleene closure at all)

Unless we have very different understandings of the Kleene closure/Kleene star/*, this...isn't true. I'm a little hesitant (you have far more experience in tcl!), so I'll be brief. Consider the regexp generated by -format {%Y-%m-%d %H:%M:%S}:

{^[[:space:]]*\s*(\d\d)(\d\d)\-\s*(\d\d?)\-\s*(\d\d?)[[:space:]]+\s*(\d\d?)\:\s*(\d\d?)\:\s*(\d\d?)\s*$}

The date parsing regexp generation inserts \s* before virtually all possible % fields (as an example, here's the source for handling %S) which makes creating a dfa that does capturing at the same time non-trivial.

[sebres]

@aidanhs Thx for the testing!

A couple of incompatibilities

Answered in sebres/tcl#2 (comment)

[aidanhs]

Although such "artificial" and very irrational "date-time" formats are theoretically imaginable, but I don't think, that it makes sense at all in the practice (or just to be 100% backwards compatible?).

This is fine (I do agree that the current matching has an excessive number of corner cases and would significantly benefit from being simplified) but I do think it's worth being aware of these (and others I might construct) simply because I know from experience that people will take whatever flexibility you give them and use/abuse it. I tend to be quite picky about 100% backwards compatibility as a result, but if the (minor) breakage is deemed to be worth the gains by the tcl team, then its an informed decision and that's fine.

[sebres]

I'm agree and I can even follow your arguments (and grateful for your demur), but as already said, many of this cases just make no sense in the practice, so I mean that the people will never take the "flexibility" in this direction, because it is simply not a date-time at all (although it would be parseable formerly).

Each extension of some existing thing may theoretically break the hundred per cent backwards-compatibility, but IMHO that's why it is not the reason to forgo the enhancements.

So again, what I mean - I'm not against your objection, it was just to clarify the facts...

[sebres]

Hello all,

I would really appreciate if leastwise somebody will give a short feedback about the topic (clock speed-up).
So far absolutelly no reaction (except @aidanhs, thx again).

If there is no intention at all as regards this PR, I'd like to know it.
Just to be sure, because I really hate the working for the recycle bin... And unfortunately it looks like it, at least in the moment the reaction of tcl-core team (how a many times before) reminds the dev/null.

Or all the people are still in the Christmas holidays?

[aidanhs]

@sebres my assumption is that flightaware are pretty pleased that you have an implementation already and will now wait for you to go through the process of submitting the patch (have you done so? Might be worth splitting up your PR as the timerate stuff could be separate) and getting it into Tcl (per the rules in the readme).

On the downside, this may well be painful from what you've said. On the upside, it sounds like you'll hit the $20k reward if you get through it.

[sebres]

Might be worth splitting up your PR

It is already so, see sebres/tcl#3

The clock speed-up is based on this branch.

[sebres]

Rebased to fossil-repository + "TIP" ATM as ticket - http://core.tcl.tk/tcl/info/ddc948cff9781daa

[martinwguy]

Greetings
To make it even faster, you can use use Algorithm 199 from "Collected Algorithms from ACM" Volume 1, which uses mathematical properties of the sequence of month lengths to convert from yearday to month/monthday without a "for month=1 to 12" loop.
Here's the original paper (sorry about the link; github is refusing to attach PDFs...)

I attach an example C file implementing mktime() and gmtime() using that algorithm, embedded in a test harness that compares their result with that of the system mktime() and gmtime() if you define TEST
mkgmtime.txt
Good luck with the bounty!

[sebres]

@martinwguy Thank you.

As regards as "for month=1 to 12" - I don't have it at all. Conversion to/from yearday (but also julianday resp. gregorian) solved pure algorithmic (arithmetically, see implicit conversions like GetJulianDayFromEraYearMonthDay or back with GetGregorianEraYearDay).

[martinwguy]

@sebres You're welcome! :)

I see in generic/tclClock.c's GetMonthDay()
for (month = 0; month < 12 && day > h[month]; ++month) { day -= h[month]; }
but maybe I am looking in the wrong place.

[sebres]

Ah now I understood what you meant.
Well, would be currently a wrong screw, because the cost of it comparable to current whole execution time of scan/format (at least 0.20 µs/#) would be to neglect. So resolving of this cycle will take currently almost nothing to account.

Just compare January and December, so full cycle here for Dec (12):

  % timerate -calibrate {} 1000
  0.048508207685756845 µs/#-overhead 0.048508 µs/# 20615068 # 20615068 #/sec
  % proc test {script} {append _ [if 1 $script] ": " [timerate $script]}
- % test {clock format 1483228800 -g 1 -f "%Y-%m-%d"}
+ % test {clock format 1483142400 -g 1 -f "%Y-%m-%d"}
- 2017-01-01: 0.281122 µs/# 3033703 # 3557173 #/sec 852.841 nett-ms
+ 2016-12-31: 0.281017 µs/# 3034675 # 3558508 #/sec 852.794 nett-ms

You see - almost resp. exact the same execution times (and differences there are measurement errors).

But I'll put it to my todo-list. Thx, again.

[resuna]

So, current status of this is... waiting on a TIP?

[sebres]

Thanks for the reminder (just too many tasks)!
Nop, I think, we don't need a TIP as long as it is not realy an enhancement (besides the little incompatibilities, which belong rather to bug fixing resp. some artifical cases).
I hoped for some feadback from TCT (or/and from flightaware;), but excepting pair colleagues I got nothing up to now from there.
Inbetween I had found a small "bug" there, that is already fixed (must be rebased in fossil).
And I've tested it on some systems of me using tcl (where it looks very good all the time).

So I'll try to make a back-porting to 8.6 the next week (should be relative easy, because I almost alone dare to change there something;), and then to start CFV survey for ca. two weeks for both 8.6 and trunk branches (I hope one or the other is accepted, or I'm listening some agruments against, that I can then "fix").

[sebres]

BTW, this RFE (instead of TIP) can be used ATM for discussion when required.

[kennykb]

Copying tcl-core on this as well:

On 04/07/2017 02:25 PM, Serg G. Brester wrote: Thanks for the reminder (just too many tasks)! Nop, I think, we don't need a TIP as long as it is not realy an enhancement (besides the little incompatibilities, which belong rather to bug fixing resp. some artifical cases). I hoped for some feadback from TCT (or/and from flightaware;), but excepting pair colleagues I got nothing up to now from there. Inbetween I had found a small "bug" there, that is already fixed (must be rebased in fossil). And I've tested it on some systems of me using tcl (where it looks very good all the time). So I'll try to make a back-porting to 8.6 the next week (should be relative easy, because I almost alone dare to change there something;), and then to start CFV survey for ca. two weeks for both 8.6 and trunk branches (I hope one or the other is accepted, or I'm listening some agruments against, that I can then "fix"). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AANPUYJBdJqeY1VEu2mWggcT8p49fvzrks5rtn-PgaJpZM4K2cWi>.

I don't recall ever getting answers to several questions that I asked. It could be that there were technological issues that kept us from making contact. (1) Are you willing to maintain the code moving forward? (If not, I need more time to review. Things have been very busy for the last few months, and I've not had time to source-dive in your separate branches. (2) What, if any, existing test cases needed to be modified to support your performance improvements, and why? (If they were there simply for 'bugward compatibility', that's an acceptable answer. I recognize that some of the Tcl test cases have historically not been tests, but rather experiments. (3) Does the code continue to support multilocalization (having places in the same process or in the same interpreter where different time zones or locales are in use)? (4) Is the claimed speedup achievable without also addressing the msgcat subsystem, or were speedups there also in scope? (5) Is the new code still self-contained, or does it rely on third-party libraries that are subject to licensing that must be reviewed? (6) Has the new code been run with the test suite under a profiler such as gcov (or nagelfar for any portions that are still written in Tcl)? What fraction of the code remains uncovered? If all of these questions have acceptable answers, then I don't think there's any need for a vote. I speak both as a TCTer and as the maintainer of record for [clock]. We just need to get you commit access for (1) - I can do that if you agree - and then get everything merged in. I presume documentation changes should be minimal, just the new %Es plus documenting any other new functionality needed. If it's a drop-in replacement that passes the test suite, I don't have a problem with it appearing in a point release. If you haven't merged recently, you'll also find that I added ensemble compilation for [clock] (and for [encoding] which was also missing it). In addition, I added bytecoding for [clock seconds/millis/micros/clicks], which should be an additional little boost. I just dropped those into core-8-6-branch as well as trunk, and nobody objected. Thanks for taking this on! I'd been meaning to do the rewrite in C (which for me would have been relatively straightforward but time-consuming) for years, but always had other fish to fry. Doing the initial implementation in C would have got me horribly bogged down, so I still think that proving the concept in Tcl first was a good idea, but somehow, the Tcl implementation always remained Almost Good Enough and the performance hacking never got enough attention.

…

-- 73 de ke9tv/2, Kevin

[kennykb]

No time currently to answer detailed, so firstly I'll try to summarize as short answer... 1) Yes 2) All the test-cases modifications, that I done, going to bug-fixing (IMHO) and to cover new functionality (like "-now" by "clock format -now -format ..."). 3) Yes (otherwise several test-cases will fail) 4) There was also in scope (no chances to get reasonable performance-increase without optimization of msgcat-clock binding). 5) Yes. It is only my own code (licensed as TCL self). 6) Yes, almost nothing uncovered (only pair code blocks serves debugging purposes). As regards of ensemble compilation for [clock], I've seen it (you've made it for commands like seconds, microseconds, etc.), but I've another branch, where ALL ensembles will be compiled (using rewritten much faster version of INST_INVOKE_REPLACE), see #21 (comment). I'll try to rebase it into fossil. Regards, sebres 08.04.2017 04:46, Kevin Kenny wrote:

Copying tcl-core on this as well: On 04/07/2017 02:25 PM, Serg G. Brester wrote: > Thanks for the reminder (just too many tasks)! Nop, I think, we don't need a TIP as long as it is not realy an enhancement (besides the little incompatibilities, which belong rather to bug fixing resp. some artifical cases). I hoped for some feadback from TCT (or/and from flightaware;), but excepting pair colleagues I got nothing up to now from there. Inbetween I had found a small "bug" there, that is already fixed (must be rebased in fossil). And I've tested it on some systems of me using tcl (where it looks very good all the time). So I'll try to make a back-porting to 8.6 the next week (should be relative easy, because I almost alone dare to change there something;), and then to start CFV survey for ca. two weeks for both 8.6 and trunk branches (I hope one or the other is accepted, or I'm listening some agruments against, that I can then "fix"). -- You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub

<#4 (comment) [1]>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AANPUYJBdJqeY1VEu2mWggcT8p49fvzrks5rtn-PgaJpZM4K2cWi [2]>.

I don't recall ever getting answers to several questions that I asked. It could be that there were technological issues that kept us from making contact. (1) Are you willing to maintain the code moving forward? (If not, I need more time to review. Things have been very busy for the last few months, and I've not had time to source-dive in your separate branches. (2) What, if any, existing test cases needed to be modified to support your performance improvements, and why? (If they were there simply for 'bugward compatibility', that's an acceptable answer. I recognize that some of the Tcl test cases have historically not been tests, but rather experiments. (3) Does the code continue to support multilocalization (having places in the same process or in the same interpreter where different time zones or locales are in use)? (4) Is the claimed speedup achievable without also addressing the msgcat subsystem, or were speedups there also in scope? (5) Is the new code still self-contained, or does it rely on third-party libraries that are subject to licensing that must be reviewed? (6) Has the new code been run with the test suite under a profiler such as gcov (or nagelfar for any portions that are still written in Tcl)? What fraction of the code remains uncovered? If all of these questions have acceptable answers, then I don't think there's any need for a vote. I speak both as a TCTer and as the maintainer of record for [clock]. We just need to get you commit access for (1) - I can do that if you agree - and then get everything merged in. I presume documentation changes should be minimal, just the new %Es plus documenting any other new functionality needed. If it's a drop-in replacement that passes the test suite, I don't have a problem with it appearing in a point release. If you haven't merged recently, you'll also find that I added ensemble compilation for [clock] (and for [encoding] which was also missing it). In addition, I added bytecoding for [clock seconds/millis/micros/clicks], which should be an additional little boost. I just dropped those into core-8-6-branch as well as trunk, and nobody objected. Thanks for taking this on! I'd been meaning to do the rewrite in C (which for me would have been relatively straightforward but time-consuming) for years, but always had other fish to fry. Doing the initial implementation in C would have got me horribly bogged down, so I still think that proving the concept in Tcl first was a good idea, but somehow, the Tcl implementation always remained Almost Good Enough and the performance hacking never got enough attention.

Links: ------ [1] #4 (comment) [2] https://github.com/notifications/unsubscribe-auth/AANPUYJBdJqeY1VEu2mWggcT8p49fvzrks5rtn-PgaJpZM4K2cWi

[kennykb]

On Mon, Apr 10, 2017 at 6:58 AM, Dipl. Ing. Sergey G. Brester < ***@***.***> wrote: No time currently to answer detailed, so firstly I'll try to summarize as short answer... 1) Yes 2) All the test-cases modifications, that I done, going to bug-fixing (IMHO) and to cover new functionality (like "-now" by "clock format -now -format ..."). 3) Yes (otherwise several test-cases will fail) 4) There was also in scope (no chances to get reasonable performance-increase without optimization of msgcat-clock binding). 5) Yes. It is only my own code (licensed as TCL self). 6) Yes, almost nothing uncovered (only pair code blocks serves debugging purposes).

Those are almost the right answers for "goes in without a TIP." The only thorny one is the "new functionality" part. Do you have a description of the new functionality in one place, that we could use as the skeleton of a TIP? I'm willing to bend the rules if the new functionality cannot readily be separated from the other improvements. If it can, the logical thing to do would be to get the performance upgrades into 8.6.x (no TIP needed) and the new stuff onto the 8.7 development line (TIP likely non-controversial, provided that it makes sense).

[kennykb]

Do you have a description of the new functionality in one place, that we could use as the skeleton of a TIP?

Yes, this [RFE](http://core.tcl.tk/tcl/info/ddc948cff9781daa) describes that. There are only: - freescan: relative date with ordinal month and relative weekday (early it was rather undefined behavior). - I've introduced an optional tokens, for example zone (`%z` resp. `%Z`) is optional now (can be switched off); - additionally scan/format token `%Es` introduced to parse or format local seconds (in opposition to `%s` for posix seconds); - value `-now` will be accepted as clock value for format or add functions, e. g.: ``` clock format -now -f %u; ``` I don't think that this minimal "enhancements" would block the release it in 8.6.

the new functionality cannot readily be separated from the other

Some things are just per design, thus not really separable (e. g. decision rules are logic-based now, no more priority-based, see ticket [e7a722cd35](http://core.tcl.tk/tcl/tktview?name=e7a722cd35) ). But I thinks, this can be classified rather as "bugs", not as "enhancements". Regards, sebres. Am 10.04.2017 20:20, schrieb Kevin Kenny:

…

On Mon, Apr 10, 2017 at 6:58 AM, Dipl. Ing. Sergey G. Brester ***@***.***> wrote: > No time currently to answer detailed, so firstly I'll try to summarize as short answer... > > 1) Yes > > 2) All the test-cases modifications, that I done, going to bug-fixing (IMHO) and to cover new functionality (like "-now" by "clock format -now -format ..."). > > 3) Yes (otherwise several test-cases will fail) > > 4) There was also in scope (no chances to get reasonable performance-increase without optimization of msgcat-clock binding). > > 5) Yes. It is only my own code (licensed as TCL self). > > 6) Yes, almost nothing uncovered (only pair code blocks serves debugging purposes). Those are almost the right answers for "goes in without a TIP." The only thorny one is the "new functionality" part. Do you have a description of the new functionality in one place, that we could use as the skeleton of a TIP? I'm willing to bend the rules if the new functionality cannot readily be separated from the other improvements. If it can, the logical thing to do would be to get the performance upgrades into 8.6.x (no TIP needed) and the new stuff onto the 8.7 development line (TIP likely non-controversial, provided that it makes sense).

[sebres]

Just to be sure, my message was arrived:
@resuna:

I've pushed interim 8.6 version, that you can test already now.

Back-ported to 8.6 now, see branch "sebres-8-6-clock-speedup".

WiP: documentation, code review (+ still one known bug in integration with msgcat-dictionary).

If I get it ready (I hope still today) I will create 2 back-merge points for trunk (re-merge back into branches "sebres-trunk-timerate" and "sebres-clock-speedup").

Right after that I'll contact KBK resp. send CFV to TCT.

BTW. Because all sub-commands compiled now (no ensemble overhead anymore for "clock ..."), the performance increase compared to original clock grows now up to 50 - 100 times.

[sebres]

So I'm actually done with sebres-8-6-clock-speedup.
I'll still reintegrate it again back into the trunk-branch (but for 8.6 it seems to be ready).

@kennykb What would you say: CFV or just notify all with "ready state".
Nevertheless that I had given lots of time for the review, anyway I would like to give the people some time for the review and short feedback, at least about new changes, before merging.
Up now I had very few feedback from there (except you, Harald and Jan).

@resuna, @flightaware Would you need something (currently or soon) from me (like help, info, suggestions etc.) as regards the clock-speedup? Just to better plan my activities, because I've still a lot several open issues and PR's with various priorities (in core.tcl, some several things mentioned in #21, etc.).

[sebres]

I merged it just now in fossil-repository of the tcl-core (into 8.6 and trunk).
So this "issue" can be closed, IMHO.

[sebres]

I would really like to know how I can go further.
My suggestion with outsource "clock" to the library?
Or should we still wait for the acceptance, as it is?
I assume just that the certain "conservatism" of some people from TCT will perish it up to "unusable" state.
Too often I leave something behind, to determine in some years - the branch is so out of date, that is hardly adaptable resp. restorable to the major branch only with huge effort.

Another good question - Is flightaware still interested at all on this bounty program? Just 'cause no reaction until now...

[resuna]

We're still definitely interested.

Isn't it really close to getting into the core? They just are concentrating on 8.6.7 right now.

[sebres]

Porting as tcl-module: sebres/tclclockmod

[sebres]

@martinwguy I do never forget my "todo's" :)
Today I had got five minutes to re-implement it, see sebres/tclclockmod#5 for the fast month-day calculation...

[sebres]

Should be closed now?

[sebres]

I've rebased (back-ported) my current state of tclclockmod back to the tcl.core in branch "sebres-8-6-clock-speedup-cr2".

In comparision to sebres-8-6-clock-speedup-cr1 branch (was at some point removed from core-8-6-branch):

• several bugs fixed (+ minor optimizations);
• test-cases extended;
• Flightaware DST fix applied, see 40ff672959 (this bug is still present in core branches);
• Recognize positive time zone offset like "31 Jan 14 23:59:59 +0100", closes 5019748c73;
• validation rules introduced, closes 3475995 (option "-valid 1|0" with possibility of setting the default configuration, more in PR Clock validate mode sebres/tclclockmod#10);

I'll continue to maintain both (the module as well as tcl-core branch) as long as not accepted from TCT.

@kennykb, @resuna Is there some points from your side?