-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Requirements for [array foreach] #27
Comments
Cross reference with this existing TIP: TIP #421 |
It was the intention of myself and Andy Goth to merge his work and mine to ensure that there would be a single C API. I think this is the best path forward, as his new API will handle the traces and error conditions properly. I would like to benchmark an 'array for' command written using his API and compare it against what I wrote. The existing TIP has not been updated. Arguments about For the
|
|
Yes, it should save a little time, but I don't think the end result is much different from the Tcl code, excepting the overhead of the set call (I could not determine any simpler way to get the value object). I will try to find some time to reimplement my code using Andy Goth's API and see how that works out. The C code does:
|
OK, so your implementation is basically a "C" version of the stock |
As I understand it, the problem is that traces can do all sorts of tricky things that cause trouble. I guess that a more subtle approach would be to use some sort of modification sequence number in the array (changing the structure of arrays) so that iterations could perform a simple comparison to detect the world changing under their feet. We use similar techniques elsewhere (e.g., with bytecode where renaming a command with a compiler increments the compilation epoch counter). |
No, that is not correct. No, it is not a stock version of Even after bytecoding, I don't think you are going to get any huge speed gains, but the memory savings are significant. |
I didn't mean to imply that it was just a stock version of Since the loop is running in "C" code it doesn't matter whether the operation itself is bytecoded, the operation lookup is a tiny fraction of the total time. |
Right. I think once bytecoded, there will some time savings, but nothing significant. The fetch of the value object from the hash table (which has all the trace checking, etc.) is never going to be any faster. |
OK, I don't understand what you're doing then, if you're walking the hash table to get the keys but you don't have access to the values in the same operation. What we're looking for is an analog to the Speed Tables "$table search" operation, which is O(N) for a straight linear search because it walks the index and has access to the row in the table without an extra lookup on the key. |
Hmm, it's been pointed out that the time for Tcl_GetVar2 shouldn't be O(logN) unless there's a serious problem with Tcl hash tables, but there's some kind of superlinear term because I can't see otherwise how the time for your |
Does |
Well, as DKF hinted, We can:
Then it could be made much faster, as the low-level hash table operations and fetch of value objects could be used. The problem is, many people will expect an |
The expectation from the bounty request was that Does |
If there are no read traces set for the array, then it should be able to tell that by checking once at the start of the loop, no? It shouldn't be duplicating that check on every row. |
It appears that |
That's a very surprising discovery, and something I will have to keep in mind when debugging Tcl code in the future because I would have assumed |
No, I was incorrect -- it fires the trace on the individual element. |
|
I'll get with karl on this, it's a very surprising result. He may still be interested in the code clean-up advantages of being able to step through the array without copying the keys. |
Maybe we need a bounty on speeding up array access in general as a follow-up. |
Let's revisit this one after Andy gets back to #18 ? |
Here is a very simple implementation of proc arrayforeach {keyvarName arrayName body} {
upvar 1 $keyvarName key $arrayName array
set s [array startsearch $array]
try {
while {[array anymore array $s]} {
set key [array nextelement array $s]
uplevel 1 $body
}
} finally {
array donesearch array $s
}
}
# Inject foreach into [array] ensemble
namespace ensemble configure ::array -map \
[dict replace [namespace ensemble configure ::array -map] \
foreach [list [namespace which arrayforeach]]] I don't think it is quite enough (it only digs out the keys), but it shows that people can test out ideas without having to commit to writing C straight off the bat. |
There are a number of things that are required to satisfy the
array foreach
bounty. Because there's interest from multiple people over this, I'll lay down what we really need.foreach
, of thearray
ensemble. (Preferably to be part of Tcl 8.7.)Tcl_EvalObjEx
, etc.tcltest
) are required. Should test simple usage, failure modes (such as wrong args, unwriteable iteration variable(s), etc.) and “interesting” edge cases like semantics when the array is written to during the iteration loop.It's not enough to just have an implementation. It must also be tested (so that we can safely develop improved implementations as required) and it must be documented (so other Tclers can learn about it). The TIP is conceptually part of the documentation, used as a form of milestone in our release notes.
The text was updated successfully, but these errors were encountered: