https://pantsbuild.org/ logo
Powered by
#development
  • e

    enough-analyst-54434

    04/02/2018, 7:25 PM
    @witty-crayon-22786 I don't think it's super-important, but it surprises me that there is no way to hide a coroutine exchange behind a python function, preserving the python/rust call boundary you have in https://github.com/pantsbuild/pants/pull/5580 but hiding it from user code.
  • w

    witty-crayon-22786

    04/02/2018, 7:28 PM
    @enough-analyst-54434: the coroutine approach has coroutines called by the rust code, and then the python code yields a request to the rust code
  • w

    witty-crayon-22786

    04/02/2018, 7:29 PM
    result = yield rust(value)
    "looks" like calling back to rust and then yielding the return value of that
  • e

    enough-analyst-54434

    04/02/2018, 7:29 PM
    and then the python code yields a request to the rust code
    I'm hung up on why that python code must be user-written rule code and not a wrapper to hide the coroutine stuff
  • w

    witty-crayon-22786

    04/02/2018, 7:29 PM
    which isn't equivalent. but i might be misreading it
  • w

    witty-crayon-22786

    04/02/2018, 7:33 PM
    @enough-analyst-54434: a coroutine can't yield through multiple levels of the callstack, afaik
  • w

    witty-crayon-22786

    04/02/2018, 7:34 PM
    so the 
    @rule
    cannot "be yielded" by something it calls into... into needs to call yield directly
  • w

    witty-crayon-22786

    04/02/2018, 7:38 PM
    but whether this matters or not is an open question... i think you're right that having 
    Rule.map
    as a method would allow for an equivalent amount of parallelism/concurrency in practice. and if calling back into rust releases the GIL, then coroutine vs synchronous would be more of a style choice rather than an efficiency choice
  • w

    witty-crayon-22786

    04/02/2018, 11:25 PM
    @enough-analyst-54434: ^ another ping, just to confirm that you've seen this
  • w

    witty-crayon-22786

    04/02/2018, 11:26 PM
    Daniel is out this week, so i won't be trying to land that patch this week. but i'm going to be trying to land something mid next week at the latest so that i can talk about it at a conference =x
  • e

    enough-analyst-54434

    04/03/2018, 2:17 PM
    @witty-crayon-22786 saw it and makes sense. It does look like the gil is released (http://cffi.readthedocs.io/en/latest/ref.html#conversions note [2]). If so / barring real perf issues, I definitely favor making rules look like 1st class values that can be passed to other rule functions, but I may be wrong in guessing this would be a simpler / more clear model for end-user rule writers; so not a super-strong objection if the coroutine syntax must stay.
  • w

    witty-crayon-22786

    04/03/2018, 3:35 PM
    @enough-analyst-54434 I think that without refactoring the entire concurrency model away from futures, we need the coroutine syntax. Because if the calls are synchronous back and forth between rust and python we can't really do async io. We could do something like pure rayon (rust forkjoin lib), which would likely work though.
  • e

    enough-analyst-54434

    04/03/2018, 3:49 PM
    Were you hiding that realization from me yesterday 😉 ! That does make sense as a ~blocker reason. Hrmph.
  • e

    enough-analyst-54434

    04/03/2018, 3:49 PM
    It would be nice to "compile" rules by sed'ing rule stub + s/return/yield/
  • w

    witty-crayon-22786

    04/03/2018, 3:50 PM
    heh
  • e

    enough-analyst-54434

    04/03/2018, 3:50 PM
    Not much crazier than parsing them!
  • w

    witty-crayon-22786

    04/04/2018, 5:55 PM
    @rough-minister-58256: have you ever seen a case where a python 
    TypeType
    wasn't equal to itself...?
    Copy code
    >>> <class 'pants.backend.graph_info.tasks.listtargets.ListTargets_list'> == <class 'pants.backend.graph_info.tasks.listtargets.ListTargets_list'> ??: False
>>> <class 'pants.backend.graph_info.tasks.listtargets.ListTargets_list'> issubclass <class 'pants.backend.graph_info.tasks.listtargets.ListTargets_list'> ??: False
>>> <class 'pants.backend.graph_info.tasks.listtargets.ListTargets_list'> issubclass <class 'pants.backend.graph_info.tasks.listtargets.ListTargets_list'> ??: False
  • w

    witty-crayon-22786

    04/04/2018, 5:55 PM
    in this case it's across the fork boundary, but i can't imagine that we're unloading any python classes during that
  • r

    rough-minister-58256

    04/04/2018, 5:55 PM
    hm, what does 
    a is b
    say?
  • w

    witty-crayon-22786

    04/04/2018, 5:56 PM
    ...and when i say "itself", what i mean is: equal to something that i expect with very high probability to be itself
  • r

    rough-minister-58256

    04/04/2018, 5:56 PM
    and 
    id(a), id(b)
  • r

    rough-minister-58256

    04/04/2018, 5:56 PM
    default equality iirc is based on id(a) == id(b)?
  • w

    witty-crayon-22786

    04/04/2018, 5:57 PM
    Copy code
    print('>>> {} == {} ??: {}'.format(task, task_cls, task == task_cls))
      print('>>> {} issubclass {} ??: {}'.format(task, task_cls, issubclass(task, task_cls)))
      print('>>> {} issubclass {} ??: {}'.format(task_cls, task, issubclass(task_cls, task)))
      print('>>> {} is {} ??: {}'.format(task_cls, task, task_cls is task))
      print('>>> {} id(x) {} ??: {} {}'.format(task_cls, task, id(task_cls), id(task)))
  • w

    witty-crayon-22786

    04/04/2018, 5:57 PM
    Copy code
    >>> <class 'pants.backend.graph_info.tasks.listtargets.ListTargets_list'> == <class 'pants.backend.graph_info.tasks.listtargets.ListTargets_list'> ??: False
>>> <class 'pants.backend.graph_info.tasks.listtargets.ListTargets_list'> issubclass <class 'pants.backend.graph_info.tasks.listtargets.ListTargets_list'> ??: False
>>> <class 'pants.backend.graph_info.tasks.listtargets.ListTargets_list'> issubclass <class 'pants.backend.graph_info.tasks.listtargets.ListTargets_list'> ??: False
>>> <class 'pants.backend.graph_info.tasks.listtargets.ListTargets_list'> is <class 'pants.backend.graph_info.tasks.listtargets.ListTargets_list'> ??: False
>>> <class 'pants.backend.graph_info.tasks.listtargets.ListTargets_list'> id(x) <class 'pants.backend.graph_info.tasks.listtargets.ListTargets_list'> ??: 140293868914912 140293866774624
  • w

    witty-crayon-22786

    04/04/2018, 5:57 PM
    so... yea. they're different.
  • w

    witty-crayon-22786

    04/04/2018, 5:57 PM
    i'm going to try and figure out... where that happens
  • r

    rough-minister-58256

    04/04/2018, 5:58 PM
    hrm
  • w

    witty-crayon-22786

    04/04/2018, 5:59 PM
    i suspect the fork boundary, because this works outside the daemon
  • r

    rough-minister-58256

    04/04/2018, 5:59 PM
    id() maps to mem address
  • r

    rough-minister-58256

    04/04/2018, 5:59 PM
    yeah
1...463464465...906Latest