Skip to content
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

Add %jinx hint to Vere. #648

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

Add %jinx hint to Vere. #648

wants to merge 7 commits into from

Conversation

sigilante
Copy link
Contributor

Resolves proposed UIP (unnumbered).

Adds a %jinx hint to Vere which permits a computation's timeout to be specified in Urbit time.

> ~>  %jinx.[~s4]  =|(i=@ |-(?:(=(10.000.000 i) i $(i +(i)))))
10.000.000

> ~>  %jinx.[~s3|  =|(i=@ |-(?:(=(10.000.000 i) i $(i +(i)))))
recover: dig: alrm
crud: %belt event failed
call: failed

@sigilante sigilante requested a review from a team as a code owner May 17, 2024 19:51
@sigilante
Copy link
Contributor Author

May want to improve the error message to provide guidance on why the crash takes place.

pkg/noun/nock.c Outdated
Comment on lines 1931 to 1945
if (c3y == u3a_is_atom(*clu)) {
// clu is in Urbit time, but we need Unix time
mpz_t clu_mp;
u3r_mp(clu_mp, *clu);
mpz_t urs_mp, tim_mp;
mpz_init(urs_mp);
mpz_init(tim_mp);
mpz_tdiv_q_2exp(tim_mp, clu_mp, 48);
mpz_mul_ui(tim_mp, tim_mp, 1000);
mpz_tdiv_q_2exp(urs_mp, tim_mp, 16);
c3_w mil_w = u3i_mp(urs_mp);
u3m_timer_set(mil_w); // set ITIMER (mil_w is in microseconds)
mpz_clear(clu_mp);
mpz_clear(tim_mp);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can replace this math with u3_time_msc_out(c3_d).

c3_d tim_d;
if ( c3y == u3r_safe_chub(*clu, &tim_d) ) {
  c3_w mic_w = u3_time_msc_out(tim_d);
  c3_w mil_w = mic_w / 1000;
  u3m_timer_set(mil_w);
}

(Or you could, if it was defined in pkg/vere. I'd be inclined to copy it into this file as a static function rather than use gmp.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Urbit time is bigger than that, though, so the u3r_safe_chub always fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could retrieve two chubs for the time, very unlikely to be too big for that.

@sigilante
Copy link
Contributor Author

notes from discussion today, for posterity:

  • mil_w was a functioning instance of this mechanism in u3m_soft for certain executions.
  • u3m_softu3m_soft_top which processes trace for you
  • signals send you to a top-level call
  • we probably have to do different things depending on whether we're in a virtual computation or not.
  • if this only ever happens in a road, and we only run u3m_soft from inside, then we can assume jump buffer is initialized and all we need to do is reset itimer_virtual in cm_signal_deep()
  • in cm_signal_deep() if mil_ != 0
  • so reset SIGVTALRM if we hit a new %jinx hint
  • fn in manage.c that nock.c can call when it encounters this hint that does the same thing that cm_signal_deep() does with the interval timer, but it also needs to check somehow
  • need some other global state to be able to see if we are prepared to handle this signal or not
  • vere/serf.c → ll. 26, 28 mil=@
    • inner timer if computation is done
    • outer timer to see if still valid
  • could build proof of concept that doesn't worry about safety and nesting
  • if you call setitimer multiple times you clobber the previous value, so to nest you need global state (timer value and monotonic timestamp), then when comp finishes then restore parent with adjusted time
  • proof of concept: explore hint callbacks in nock.c, whitelist in bytecode compiler, then check dynamic hint processing
    case c3__jinx: {
      if (c3y == u3a_is_cat(*clu)) {
        c3_w mil_w = *clu;
        // call fn in u3m passing it mil_w TODO
      }
      u3z(*clu);
      *clu = u3_nul;
    } break;

one to set ITIMER like cm_signal_deep, one to cancel it using zeroed itervaltimerstruct

@tacryt-socryp
Copy link
Contributor

Extremely based functionality

@sigilante
Copy link
Contributor Author

From discussion ~2024.8.12, some thoughts on the interface:

  • subsecond syntax for @dr, like ~us1000? ~ms, ~us, ~ns (mibiseconds as powers of two?)
  • (ms 1000) support function?
  • use bloq size instead?

So write a UIP to implement subsecond timing syntax?

kiki-second, miki-second, niki-second, piki-second

@sigilante
Copy link
Contributor Author

Probably just stick with @dr and add ++msec, ++usec arms for now.

@sigilante
Copy link
Contributor Author

Ongoing support work for Urbit @dr time here: urbit/urbit#7057

@sigilante
Copy link
Contributor Author

Also urbit/urbit#7067 for the parser.

@sigilante
Copy link
Contributor Author

Should the nested timer be measured against the nominal outer timer or against the actual time remaining? I'm building the stack handler for the latter case, but worth thinking about.

@sigilante
Copy link
Contributor Author

::  should fail (timeout)
~>  %jinx.[~s0..8000]  =|(i=@ |-(?:(=(10.000.000 i) i $(i +(i)))))

::  should succeed (normal)
~>  %jinx.[~s1]  =|(i=@ |-(?:(=(10.000.000 i) i $(i +(i)))))

::  should succeed (nested properly)
~>  %jinx.[~s2..8000]  ~>  %jinx.[~s1]  =|(i=@ |-(?:(=(10.000.000 i) i $(i +(i)))))

::  should fail (nested improperly)
~>  %jinx.[~s1..8000]  ~>  %jinx.[~s5]  =|(i=@ |-(?:(=(10.000.000 i) i $(i +(i)))))

::  should fail (nested improperly and timeout)
~>  %jinx.[~s0..8000]  ~>  %jinx.[~s1]  =|(i=@ |-(?:(=(10.000.000 i) i $(i +(i)))))

%jinx squats on the SIGVTALRM signal, and when this is caught in manage.c it always assumes that the cause is an expired timer. While this is correct for the current usage of Vere, I highlight it in case some future runtime engineer decides to utilize that signal in some other way.

@sigilante
Copy link
Contributor Author

At this point, it would be helpful to have someone else verify the behavior. There seems to be an incorrect fall-through case on ~s0 still, and I suspect that the timer stack may not always clear correctly.

Test Cases

Since the failure of a %jinx hint results in a nondeterministic error, we cannot test them using in-Arvo means. However, the following should all work correctly:

::  should fail (timeout)
~>  %jinx.[~s1]  =|(i=@ |-(?:(=(10.000.000 i) i $(i +(i)))))

::  should succeed (normal)
~>  %jinx.[~s1]  =|(i=@ |-(?:(=(10.000 i) i $(i +(i)))))

::  should succeed (nested properly)
~>  %jinx.[~s3..8000]  ~>  %jinx.[~s3]  =|(i=@ |-(?:(=(10.000.000 i) i $(i +(i)))))

::  should warn (nested improperly; will succeed on outer hint)
~>  %jinx.[~s1..8000]  ~>  %jinx.[~s5]  =|(i=@ |-(?:(=(10.000.000 i) i $(i +(i)))))

::  should warn (nested improperly; will timeout on outer hint)
~>  %jinx.[~s0..8000]  ~>  %jinx.[~s1]  =|(i=@ |-(?:(=(10.000.000 i) i $(i +(i)))))

::  should fail (timeout)
~>  %jinx.[~s0]  =|(i=@ |-(?:(=(10.000.000 i) i $(i +(i)))))

Developer Notes

  1. %jinx squats on the SIGVTALRM signal, and when this is caught in manage.c it always assumes that the cause is an expired timer. While this is correct for the current usage of Vere, I highlight it in case some future runtime engineer decides to utilize that signal in some other way.

  2. %jinx simply returns on an invalid nesting, rather than clearing the timer stack.

  3. The fast way to build this:

    bazel build :urbit --copt=-DU3_MEM_DEBUG
    ./bazel-bin/pkg/vere/urbit zod

@sigilante sigilante requested a review from pkova October 28, 2024 21:24
@sigilante
Copy link
Contributor Author

@pkova @mopfel-winrux wants me to bug you on this for 3.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants