Skip to content
Commit 8e8c7228 authored by Ralf Jung's avatar Ralf Jung
Browse files

change IntoVal so that it is easier to use in specs

parent bf9fd4f5
Loading
Loading
Loading
Loading
  • Author Owner

    Interestingly, this change (or rather, https://gitlab.mpi-sws.org/FP/iris-coq/compare/a2dab2fb4959...a4731f4c73b5) made atomic_heap.v and par.v almost double their compile time:

    https://coq-speed.mpi-sws.org/d/Ne7jkX6kk/coq-speed?orgId=1&from=1529316366080&to=1529324554450&var-metric=time&var-project=iris&var-branch=gen_proofmode&var-config=coq-8.8.0&var-group=(.*)

    Zooming out shows that this is consistent and not just noise.

    However, heap_lang/lifting.v (which uses IntoVal much more often!) is unaffected. What?!?

    Cc @robbertkrebbers

    Edited by Ralf Jung
  • Author Owner

    And here is why:

    Time rewrite /IntoVal; iIntros (<-). (* Finished transaction in 0.001 secs *)
    Undo.
    Time iIntros (<-). (* Finished transaction in 2.085 secs *)
  • Robbert Krebbers @robbertkrebbers ·
    Owner

    In the case of par, we are basically performing a rewrite on:

    IntoVal e (f1,f2)

    this thing is not directly an equality, so Coq's rewrite (which is used by the <- introduction pattern) somehow first tries to see if it can use this thing as a setoid relation.

    If you enable Typeclasses eauto := debug, so see its awful attempt.

    Edited by Robbert Krebbers
  • Robbert Krebbers @robbertkrebbers ·
    Owner

    IMHO, let's revert this whole change. The mount of porting it took to adapt existing developments was also not really worth the effort.

  • Author Owner

    Here's the slow TC trace: https://gitlab.mpi-sws.org/snippets/173

    The fast one doesn't even trigger TC search.

  • Author Owner

    The mount of porting it took to adapt existing developments was also not really worth the effort.

    Except that that work is already done.

  • Robbert Krebbers @robbertkrebbers ·
    Owner

    Except that that work is already done.

    There are surely developments by other people that have not been ported.

    Edited by Robbert Krebbers
  • Robbert Krebbers @robbertkrebbers ·
    Owner

    The fast one doesn't even trigger TC search.

    The problem is, I think, that only IntoVal e (f1,f2) can be turned into a relation on the same type, namely λ v1 v2, IntoVal e (v1, v2), whereas for the others, that is not the case.

  • Author Owner

    IntoVal doesn't even appear in that TC trace so I am wondering what the heck it is doing.

    The problem is, I think, that only IntoVal e (f1,f2) can be turned into a relation on the same type, namely λ v1 v2, IntoVal e (v1, v2), whereas for the others, that is not the case.

    I don't think so, atomic_heap also got slower and it only uses IntoVal e v.

  • Author Owner

    On the other hand, it is looking for something about rtc?!???

    4.1-1.2-2.1-1.1-3.1-2 : (subrelation (bi_entails ==> bi_entails)%signature
                               (rtc ?Goal0 --> flip ?r)%signature)

    EDIT: Ah, here

    Debug: 1.1-1.3-1.1-1.1-1 : (Reflexive ?r)
    Debug: 1.1-1.3-1.1-1.1-1: looking for (Reflexive ?r) with backtracking
    Debug: 1.1-1.3-1.1-1.1-1.1: simple apply @rtc_reflexive on (Reflexive ?r), 0 subgoal(s)

    Should Reflexive have a Hint Mode?

    Edited by Ralf Jung
  • Author Owner

    After adding

    Hint Mode Reflexive ! ! : typeclass_instances.

    The time is down from 2s to 0.4s.

  • Author Owner

    Ouch, but there is a use of setoid_rewrite which fails with that mode. Wtf.

    EDIT: That setoid_rewrite indeed uses rtc for some intermediate result... that's probably discarded again later. That doesn't seem right.

    Edited by Ralf Jung
  • Author Owner

    Interestingly,

    Remove Hints rtc_reflexive : typeclass_instances.

    also reduces the time to 0.4s. It then picks iff_reflexive instead which doesn't introduce a new evar, which seems to help. It may be worth reducing the priority of rtc_reflexive, not just for this case.

    Edited by Ralf Jung
  • Author Owner

    https://gitlab.mpi-sws.org/robbertkrebbers/coq-stdpp/merge_requests/38 decreased the time the rewrite takes from 2s to 0.38s.

  • Robbert Krebbers @robbertkrebbers ·
    Owner

    Good to hear this had an effect, but IMHO these rewrites should take something much closer to 0.

0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment