Recording the included files (from read-file-form and read-file-line) has so far failed to solve the problem of ASDF missing updates to the included files.
I could use a little advice here. Running the test I have pushed, I tried tracing asdf/action:action-up-to-date-p. It is not called during the second invocation of (asdf:load-system "test-include")
So at the moment I do not see how the flow of control works to decide whether or not it is necessary to redo the DEFINE-OP.
And maybe DEFINE-OP isn't the operation that should be re-triggered by changes to the system definition file or included files? Is the right place for these new dependencies on included files DEFINE-OP or PREPARE-OP?
thanks, r
On Fri, Jun 30, 2017 at 10:05 AM, Robert Goldman rpgoldman@sift.net wrote:
Recording the included files (from read-file-form and read-file-line) has so far failed to solve the problem of ASDF missing updates to the included files.
First, can you confirm that the compute-action-stamp called by mark-operation-done called by the perform :after method on define-op returns the correct value?
I suspect it may fail to take into account the input files, because you compute the input-files too early and the value without your addition gets cached.
If you didn't change the perform define-op method to call system-source-file directly instead of input-files, that might be the issue. And/or if your input-files fails to consult a slot from the system object that persists across sessions. Of course, you need to invalidate that slot if and when you invalidate the .asd file itself.
I could use a little advice here. Running the test I have pushed, I tried tracing asdf/action:action-up-to-date-p. It is not called during the second invocation of (asdf:load-system "test-include")
I admit the asdf model has fallen out of my cache and I can't even rebuild the context in which this trace would (or wouldn't) make sense.
So at the moment I do not see how the flow of control works to decide whether or not it is necessary to redo the DEFINE-OP.
My understanding is that the first time over, of course you do it, and the second time over, traverse-action calls compute-action-status that calls compute-action-stamp, and that decides (using notably input-files) whether the action is up to date.
And maybe DEFINE-OP isn't the operation that should be re-triggered by changes to the system definition file or included files? Is the right place for these new dependencies on included files DEFINE-OP or PREPARE-OP?
I'm pretty sure it's DEFINE-OP and not PREPARE-OP.
Thanks a lot for getting into the code. The code definitely needs more people looking into it. Also, if you find that my comments are insufficient or incorrect, they should be fixed.
I did a "code walkthrough" a few years back, but obviously it doesn't cover the new DEFINE-OP infrastructure.
—♯ƒ • François-René ÐVB Rideau •Reflection&Cybernethics• http://fare.tunes.org Reality must take precedence over public relations, for Mother Nature cannot be fooled. — R.P. Feynman
On 6/30/17 Jun 30 -10:11 AM, Faré wrote:
First, can you confirm that the compute-action-stamp called by mark-operation-done called by the perform :after method on define-op returns the correct value?
Hm. I don't see any such after method. All I see is this:
(defmethod perform ((o define-op) (s system)) (assert (equal (coerce-name s) (primary-system-name s))) (nest (if-let ((pathname (first (input-files o s))))) (with-standard-io-syntax) (let ((*print-readably* nil) ;; Note that our backward-compatible *readtable* is ;; a global readtable that gets globally side-effected. Ouch. ;; Same for the *print-pprint-dispatch* table. ;; We should do something about that for ASDF3 if possible, or else ASDF4. (*readtable* *readtable*) (*print-pprint-dispatch* *print-pprint-dispatch*) (*package* (find-package :asdf-user)) (*default-pathname-defaults* ;; resolve logical-pathnames so they won't wreak havoc in parsing namestrings. (pathname-directory-pathname (physicalize-pathname pathname))))) (handler-bind (((and error (not missing-component)) #'(lambda (condition) (error 'load-system-definition-error :name (coerce-name s) :pathname pathname :condition condition)))) (asdf-message (compatfmt "~&~@<; ~@;Loading system definition~@[ for ~A~] from ~A~@:>~%") (coerce-name s) pathname) ;; dependencies will depend on what's loaded via definition-dependency-list (unset-asdf-cache-entry `(component-depends-on ,o ,s))) (load* pathname :external-format (encoding-external-format (detect-encoding pathname)))))
This looks like it should do the right thing, but I don't know. At any rate, I'm having a hard time tracking down exactly where we determine whether or not a define-op needs redoing. The caching code makes things much more efficient, but it also effectively camouflages the algorithm....
Cheers, r
best, r
On 6/30/17 Jun 30 -10:11 AM, Faré wrote:
First, can you confirm that the compute-action-stamp called by mark-operation-done called by the perform :after method on define-op returns the correct value?
Hm. I don't see any such after method. All I see is this:
(defmethod perform ((o define-op) (s system))
...
(if-let ((pathname (first (input-files o s)))))
... This call to input-files is problematic, as it will cause the wrong value to be cached, before you recorded your input-files. Note that you might have to invalidate the cached input-files anyway, because it may already have been called by compute-action-stamp at that point.
But I meant the general perform :after method and mark-operation-done method from action.lisp.
At any rate, I'm having a hard time tracking down exactly where we determine whether or not a define-op needs redoing.
Ultimately, it boils down to (compute-action-stamp plan operation component [nil]) as called by compute-action-status in plan.lisp. And *that* will call input-files.
The caching code makes things much more efficient, but it also effectively camouflages the algorithm....
Not just "more efficient", but in some cases keeps O(n) what might be a much higher polynomial (or exponential?) when a module's input-files recursively depend on its contents (e.g. for warning checking). But yes, sometimes you need to invalidate the cache, and this looks like one of those times to me.
—♯ƒ • François-René ÐVB Rideau •Reflection&Cybernethics• http://fare.tunes.org Too many people are thinking of security instead of opportunity. They seem more afraid of life than death. — James F. Byrnes
On 6/30/17 Jun 30 -10:35 AM, Faré wrote:
On 6/30/17 Jun 30 -10:11 AM, Faré wrote:
First, can you confirm that the compute-action-stamp called by mark-operation-done called by the perform :after method on define-op returns the correct value?
Hm. I don't see any such after method. All I see is this:
(defmethod perform ((o define-op) (s system))
...
(if-let ((pathname (first (input-files o s)))))... This call to input-files is problematic, as it will cause the wrong value to be cached, before you recorded your input-files. Note that you might have to invalidate the cached input-files anyway, because it may already have been called by compute-action-stamp at that point.
But I meant the general perform :after method and mark-operation-done method from action.lisp.
OK, I see this:
(defmethod perform :after ((o operation) (c component)) (mark-operation-done o c))
That's going to compute a time stamp for the operation. I think that's OK, because that will be done after the first time the DEFINE-OP is executed. Ideally, when we go to do it the second time, this timestamp will be compared with the last write time on the input files (in this case a revision-number.lisp-expr file), and should trigger the DEFINE-OP to be done again.
But obviously I'm missing something, since this is not happening. Actually, there are a bunch of things I don't understand, including:
1. When I invoke LOAD-OP the second time, why is ACTION-UP-TO-DATE-P never called? What short-circuits this?
2. COMPONENT-OPERATION-TIME is called on DEFINE-OP of SYSTEM at what looks like the right time (as well as being called a bunch of other times, presumably to check that it's up-to-date with respect to downstream components).
3. Looks like COMPUTE-ACTION-STATUS calls COMPUTE-ACTION-STAMP. Looks like that is where we compute whether or not the operation is up-to-date wrt the input files.
4. This is likely to be the problem: we compare the time stamp of the input files against the earliest time stamp of the output files. But there are no output files from the define-op, so we do UP-TO-DATE-P = (STAMP<= <number> NIL) and STAMP<= is defined to always be true if the second argument is null.
So there is no point in the algorithm -- or at least no point that I can see -- where we compare the input-file time stamps against the component-operation-time.... I could butcher that in, but I'm a little worried -- the logic in COMPUTE-ACTION-STAMP is quite subtle.
Anyway, that's how I'm proceeding.
At any rate, I'm having a hard time tracking down exactly where we determine whether or not a define-op needs redoing.
Ultimately, it boils down to (compute-action-stamp plan operation component [nil]) as called by compute-action-status in plan.lisp. And *that* will call input-files.
The caching code makes things much more efficient, but it also effectively camouflages the algorithm....
Not just "more efficient", but in some cases keeps O(n) what might be a much higher polynomial (or exponential?) when a module's input-files recursively depend on its contents (e.g. for warning checking). But yes, sometimes you need to invalidate the cache, and this looks like one of those times to me.
—♯ƒ • François-René ÐVB Rideau •Reflection&Cybernethics• http://fare.tunes.org Too many people are thinking of security instead of opportunity. They seem more afraid of life than death. — James F. Byrnes
Almost there....
Remaining problem: when I compare the (new) write time of the version.lisp-expr file, against the component-operation-time, they are equal.
So most likely, I'm getting that time because the check is being done while I'm updating the component-operation-time, or the component-operation-time is updated at the start of computing the action stamp.
So I feel very close, but am still wandering in the wilderness....
On 6/30/17 Jun 30 -12:51 PM, Robert Goldman wrote:
Almost there....
Remaining problem: when I compare the (new) write time of the version.lisp-expr file, against the component-operation-time, they are equal.
So most likely, I'm getting that time because the check is being done while I'm updating the component-operation-time, or the component-operation-time is updated at the start of computing the action stamp.
So I feel very close, but am still wandering in the wilderness....
No, it turns out that there is no bug remaining, except for the limited resolution of the time stamps. If I put (SLEEP 2) between loading and writing the new version.lisp-expr file, all is well. I will tidy up a little and push my patch.
It seems inelegant, and also I'm not very confident in how I apportioned functions to the different ASDF component files/packages. I don't richly understand the dependency structure laid out in asdf.asd, so there was a little trial and error involved in code placement.
Cheers, r
- When I invoke LOAD-OP the second time, why is ACTION-UP-TO-DATE-P
never called? What short-circuits this?
ACTION-UP-TO-DATE-P only applies to your DEFSYSTEM-DEPENDS-ON dependencies. If you have none, it won't be called. What will be called in your case is the regular COMPUTE-ACTION-STATUS -> COMPUTE-ACTION-STAMP. What ACTION-UP-TO-DATE-P does, compared to TRAVERSE-ACTION, is query whether the dependency is up-to-date *without* marking it needed if it is out-of-date (because it might not actually be needed anymore after the system that had the defsystem-depends-on is invalidated).
- COMPONENT-OPERATION-TIME is called on DEFINE-OP of SYSTEM at what
looks like the right time (as well as being called a bunch of other times, presumably to check that it's up-to-date with respect to downstream components).
Good! That's what matters. Does that timestamp indeed include that of your read-file-line input files?
- Looks like COMPUTE-ACTION-STATUS calls COMPUTE-ACTION-STAMP. Looks
like that is where we compute whether or not the operation is up-to-date wrt the input files.
Yes! That's totally the intended process.
- This is likely to be the problem: we compare the time stamp of the
input files against the earliest time stamp of the output files. But there are no output files from the define-op, so we do UP-TO-DATE-P = (STAMP<= <number> NIL) and STAMP<= is defined to always be true if the second argument is null.
The DEFINE-OP should still have a previous COMPONENT-OPERATION-TIME, and checking that it matches the computed stamp is the last step of COMPUTE-ACTION-STAMP (or else the previous performance is invalidated).
Actually, this suggests a performance bug whereby ASDF does over recomputation: if you perform a (not needed-in-image) action, then it gets out-of-date, but some *other process* performs and makes it up to date, then we do not need to look at the COMPONENT-OPERATION-TIME!
So there is no point in the algorithm -- or at least no point that I can see -- where we compare the input-file time stamps against the component-operation-time.... I could butcher that in, but I'm a little worried -- the logic in COMPUTE-ACTION-STAMP is quite subtle.
Don't butcher anything: this function was years in the making. We (1) infer a timestamp from inputs and dependencies, and (2) compare this timestamp to what was previously performed, to see if it must be done again. But indeed this second check is only meaningful for needed-in-image actions.
It seems inelegant, and also I'm not very confident in how I apportioned functions to the different ASDF component files/packages. I don't richly understand the dependency structure laid out in asdf.asd, so there was a little trial and error involved in code placement.
Did you add a new file?
—♯ƒ • François-René ÐVB Rideau •Reflection&Cybernethics• http://fare.tunes.org Statism is the secular version of salvation through faith: it doesn't matter what bureaucrats do, only that they do it with good intentions.
-
Faré -
Robert Goldman