Yit Phang Khoo committed e35ad7e Merge

Merge from default.

Comments (0)

Files changed (1)


                 d.flag <- Dirty;
                 IFDEF ADAPTON_LOG THEN Log.usr_hooks.Log.output_graph () END ;
-              end else
+              end else (* either Clean or Obsolete *)
  1. Matthew Hammer

    This function processes edges that are Clean, marking them dirty. In the else case above, the edge is not Clean. So, the comment should read "either Dirty or Obsolete", correct?

             end d ds end
         | [] ->
                       d.flag <- Clean; (* Need to reset dirty flag here, resetting beforehand is not enough. *)
                     end else (
+                      assert (d.flag == Clean);
    1. Yit Phang Khoo author

      Yes, this is intentional: it traverses the forward edges (stored in a list), which are never obsolete since they are always recreated during re-evaluation. Whereas the backward edges (stored in a (weak-)set) can be obsolete, because they also consists of old edges from the forward list that are discarded at the beginning of re-evaluation. Also note that in lines 163-169, Obsolete edges may be returned by Dependents.merge (this may saves some churn in the set, could be another thing to measure for PEPM), but they are immediately marked Clean then.

      1. Matthew Hammer

        OK, everything you say above makes sense. Thanks for the clarifications.

        I've been experimenting with weak references for the nominal thunks, and in doing so, this assertion failed. Since what you say makes sense, I'll assume that the problem is elsewhere. I'm still extremely confused about what is going wrong when I use weak references with nominal thunks.

        1. Matthew Hammer

          Update: I figured out the bug(s) regarding weaksets.

          First, I had misinterpreted your implementation of WeakSet when I made the strong variant, which I've been using for nominal adapton. Second, I was using my implementation in a way that worked out fine for my implementation, but was goofed-up for yours. Switching back to your weak sets was indeed dropping dependencies as if they were no longer live (because they weren't technically).

          My misinterpretation was on the meaning of the "merge" operation. Your bitbucket comment above clarified this for me. I was thinking that the merge operation always returned the argument value; in fact, it returns the older one already in the set, when it exists.

          I've committed the fix, at the tail end of a bunch of misc clean-up and testing commits.

          Thanks for all the comments/discussion today, it really helped out!

          1. Yit Phang Khoo author

            Great! The code needs more comments, that's for sure, and your comments are really helpful to point me to places to focus on first :-) I'll keep cleaning up the default branch every now and then.

            BTW, WeakSet implements a subset of OCaml's built-in Weak module, so you can just rename WeakSet.Make to Weak.Make if it helps to isolate bugs.

                       repair_deps ds
                 | [] -> (