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

Split main.rs into lib.rs and bin/scryer-prolog.rs #838

Merged
merged 6 commits into from
Feb 28, 2021

Conversation

Skgland
Copy link
Contributor

@Skgland Skgland commented Feb 25, 2021

As requested by Issues #225, #570 and #765 this makes it possible to use scryer-prolog as a library,
currently this only exposes a minimal API to get the binary and some added tests working.

@triska
Copy link
Contributor

triska commented Feb 25, 2021

Impressive! Thank you a lot for working on this!

I hope this makes it possible and attractive to embed Scryer Prolog more easily in Rust applications.

@Skgland
Copy link
Contributor Author

Skgland commented Feb 25, 2021

@triska I think further work is necessary to make the library useful but this will at least get the work started

@ghost
Copy link

ghost commented Feb 26, 2021

I'm not sure allowing dead_code is the way to fix the CI.

The crate nix could be a dependency of the binary only, the same for the function handle_sigint.

@Skgland
Copy link
Contributor Author

Skgland commented Feb 26, 2021

The signal handler is now moved to the binary. Should I move the lib to its own package so that dependencies can be specified independently of the bin or is this ok?

Instead of adding the #[allow(dead_code)] I commented out the unused variant, as OptArgIndexKeyType appears to mirror OptArgIndexKey where the List variant is used, so I wasn't sure whether it is not yet used or it turned obsolete and should simply be removed.

@ghost
Copy link

ghost commented Feb 26, 2021

Should I move the lib to its own package so that dependencies can be specified independently of the bin or is this ok?

Crate like nix is specific to the binary and rustyline could be another one, for now this is fine.
Commenting the List variant shouldn't be a big issue.

I'm sorry, I should have reviewed everything. This should be the last remark, the workaround introduced by
5e93766 isn't required, the removal of PartialEq was done by e7cf5d4 due to #817. You can revert the PartialEq change and the tests will work.

@Skgland
Copy link
Contributor Author

Skgland commented Feb 26, 2021

Removed the commit that introduced the workaround and reverted the removal of PartialEq on Token

* makes most pub things in src/ pub(crate) as not to expose things accidentally
  * only those things needed by src/bin/scryer-prolog.rs and tests/scryer.rs
    should be pub
* split src/main.rs into src/lib.rs and src/bin/scryer-prolog.rs
* add tests folder and run most of the files in src/tests with cargo test
  added bytes method to Stream in src/machine/streams.rs to check if stdout is as expected
only commenting out the unused variant as `OptArgIndexKeyType`
as it appears to mirror `OptArgIndexKey` and there List is not unused
so this appears to be not yet used rather than obsolete
@Skgland
Copy link
Contributor Author

Skgland commented Feb 28, 2021

Rebased to fix conflicts

@mthom mthom merged commit 81ecd17 into mthom:rebis-dev Feb 28, 2021
@mthom
Copy link
Owner

mthom commented Feb 28, 2021

Great, thanks very much!

@leehambley
Copy link

leehambley commented Jan 19, 2022

I am watching this work closely, I have naively tried to embed Scryer in my own Rust program (in a tokio task) and would like to report the following challenges:

  1. Startup time of Machine::new(input, output.clone(), error) is on the order of 5+ seconds)
  2. The Streams (as they exist on the main branch) are less than ideal, as I believe only Bytes is suitable for continuous writing (and it requires the caller to do a lot of work to set-up channels or threads to write to that byte buffer from another context), over time also memory usage might be a concern.
  3. When using it in a Tokio context, the Machine instance is not Send, see below

I'm very thankful that such a comprehensive prolog interpreter is built in pure Rust, and I'm on my first handful of hours working with Scryer, so please understand my feedback through that lens.

Perhaps I should try again with this branch, and report my findings.

    = help: the trait `Send` is not implemented for `(dyn machine::machine_state::CallPolicy + 'static)`
    = help: the trait `Send` is not implemented for `(dyn machine::machine_state::CutPolicy + 'static)`
    = help: within `impl Future`, the trait `Send` is not implemented for `*const u8`
    = help: within `impl Future`, the trait `Send` is not implemented for `Rc<BTreeMap<machine::machine_indices::OrderedOpDirKey, (usize, u32)>>`
    = help: within `impl Future`, the trait `Send` is not implemented for `Rc<Cell<(usize, u32)>>`
    = help: within `impl Future`, the trait `Send` is not implemented for `Rc<RefCell<HashSet<Rc<std::string::String>>>>`
    = help: within `impl Future`, the trait `Send` is not implemented for `Rc<RefCell<machine::streams::InnerStream>>`
    = help: within `impl Future`, the trait `Send` is not implemented for `Rc<rug::integer::big::Integer>`
    = help: within `impl Future`, the trait `Send` is not implemented for `Rc<rug::rational::big::Rational>`
    = help: within `impl Future`, the trait `Send` is not implemented for `Rc<std::string::String>`
    = help: within `machine::machine_indices::CodeIndex`, the trait `Send` is not implemented for `Rc<Cell<machine::machine_indices::IndexPtr>>`

Pseudo reproduction how I am trying to embed scryer

The reasoning behind using Tokio this way is that my parser will constantly receive user input in a prolog-like language, which will be parsed, and canonicalized and fed to a prolog engine (Scryer, in this evolution), which will then be queried, so the pipeline goes user edits -> (partial) update the parser -> compile the ast to prolog -> query prolog vm

struct Engine {
    parser_edits: Sender<String>,
    prolog_expr: Sender<String>,
    stop: tokio::sync::broadcast::Sender<()>,
    stopper: tokio::sync::broadcast::Receiver<()>,
}

impl Engine {
    pub fn new() -> Result<Arc<Self>, Error> {
        let (stop, stopper) = tokio::sync::broadcast::channel(16);

        let (parser_edits, edit_rx) = tokio::sync::mpsc::channel(16);
        let (prolog_exprs, prolog_rx) = tokio::sync::mpsc::channel(16);

        let Engine = Self {
            parser_edits,
            prolog_exprs,
            stop,
            stopper,
        };

        let Engine = Arc::new(Engine);

        tokio::task::spawn(Engine.clone().start_parser(stop.subscribe(), edit_rx));
        tokio::task::spawn(Engine.clone().start_wam(stop.subscribe(), prolog_rx));

        Ok(Engine)
    }

    async fn shutdown(self: Arc<Self>) -> Result<(), Error> {
        self.stop.send(());
        // ..snip..
        Ok(())
    }

    async fn start_parser(
        self: Arc<Self>,
        mut stop: tokio::sync::broadcast::Receiver<()>,
        mut rx: Receiver<String>,
    ) {
        // .. snip ..
        loop {
            tokio::select! {
                _ = stop.recv() => {
                    // ..snip..
                    break
                }
                Some(msg) = rx.recv() => {
                    // ..snip.. 
                }
            }
        }
    }

    async fn start_wam(
        self: Arc<Self>,
        mut stop: tokio::sync::broadcast::Receiver<()>,
        mut rx: Receiver<String>,
    ) {
        let file = "src/tests/helloworld.pl";
        let input = Stream::from("");
        let output = Stream::from(String::new());
        let error = Stream::from(String::new());
        let mut wam = self.tracer.in_span("initializing wam", |_cx| {
            // https://github.com/mthom/scryer-prolog/blob/master/tests/scryer/helper.rs
            Machine::new(input, output.clone(), error)
        });
        self.tracer.in_span("parsing test file", |_cx| {
            wam.load_file(
                file.into(),
                Stream::from(
                    std::fs::read_to_string(AsRef::<std::path::Path>::as_ref(file)).unwrap(),
                ),
            );
        });
        loop {
            tokio::select! {
                _ = stop.recv() => {
                    println!("stopping wam because stop signal received");
                    break
                }
                Some(msg) = rx.recv() => {
                    println!("received tokens = {:?}", msg);
                }
            }
        }
        println!("wam stopped");
    }
}

let e = Engine.new
e.shutdown();

@triska
Copy link
Contributor

triska commented Jan 19, 2022

@leehambley: Thank you a lot for trying this! Could you please try this with the current rebis-dev branch? This is where all current development takes place, it contains significant changes which also make Scryer Prolog a lot faster.

Please see the recent announcement and its discussion.

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.

4 participants