Rust Task Queue











up vote
1
down vote

favorite












The purpose of this program is to have one thread (main thread) working on I/O reading lines from a file and feeding it to a pool of worker threads whose job is to perform some processing on each line provided. In this case, the processing is running the String.contains() method.



Please note that as test input, I used a giant .txt file of English dictionary words found here:



Also note that you need to specify the file name of the file to be scanned as the sole command line argument to the program. I am mostly concerned with the threading model and multithreading implementation but welcome other feedback as well; new Rust programmer.



spmc crate ver is "0.2.2" for your cargo.toml



extern crate spmc;
use spmc::channel;
use std::thread;
use std::io::BufReader;
use std::fs::File;
use std::io::BufRead;
use std::u32::MAX;
use std::env;
use std::sync::Arc;

fn main() -> Result<(), std::io::Error>
{
let args: Vec<String> = env::args().collect();
match args.len()
{
2 => {},
_ => return Err(std::io::Error::new(std::io::ErrorKind::Other, "Incorrect number of args"))
}
let filename = &args[1] as &str;
let f1 = File::open(filename)?;

let mut br = BufReader::new(f1);

let mut vecData: Vec<String> = Vec::new();

let (tx, rx) = spmc::channel();

let mut handles = Vec::new();
for n in 0..5 {
let rx = rx.clone();
handles.push(thread::spawn(move || {
loop
{
let mut line_to_check: Arc<String> = rx.recv().unwrap();
if line_to_check.contains("test")
{
println!("HIT: {}", line_to_check);
}
}

}));
}
let mut input_str = String::new();
let mut bytes_read: usize = 1;
while bytes_read != 0 {
let mut is_copy = input_str.clone();
bytes_read =
match br.read_line(&mut is_copy)
{
Ok(num) => num,
Err(err) => return Err(std::io::Error::new(std::io::ErrorKind::Other, "read_line failed...n"))
};

let str_arc : Arc<String> = Arc::new(is_copy);
tx.send(str_arc);
}


Ok(())
}









share|improve this question




























    up vote
    1
    down vote

    favorite












    The purpose of this program is to have one thread (main thread) working on I/O reading lines from a file and feeding it to a pool of worker threads whose job is to perform some processing on each line provided. In this case, the processing is running the String.contains() method.



    Please note that as test input, I used a giant .txt file of English dictionary words found here:



    Also note that you need to specify the file name of the file to be scanned as the sole command line argument to the program. I am mostly concerned with the threading model and multithreading implementation but welcome other feedback as well; new Rust programmer.



    spmc crate ver is "0.2.2" for your cargo.toml



    extern crate spmc;
    use spmc::channel;
    use std::thread;
    use std::io::BufReader;
    use std::fs::File;
    use std::io::BufRead;
    use std::u32::MAX;
    use std::env;
    use std::sync::Arc;

    fn main() -> Result<(), std::io::Error>
    {
    let args: Vec<String> = env::args().collect();
    match args.len()
    {
    2 => {},
    _ => return Err(std::io::Error::new(std::io::ErrorKind::Other, "Incorrect number of args"))
    }
    let filename = &args[1] as &str;
    let f1 = File::open(filename)?;

    let mut br = BufReader::new(f1);

    let mut vecData: Vec<String> = Vec::new();

    let (tx, rx) = spmc::channel();

    let mut handles = Vec::new();
    for n in 0..5 {
    let rx = rx.clone();
    handles.push(thread::spawn(move || {
    loop
    {
    let mut line_to_check: Arc<String> = rx.recv().unwrap();
    if line_to_check.contains("test")
    {
    println!("HIT: {}", line_to_check);
    }
    }

    }));
    }
    let mut input_str = String::new();
    let mut bytes_read: usize = 1;
    while bytes_read != 0 {
    let mut is_copy = input_str.clone();
    bytes_read =
    match br.read_line(&mut is_copy)
    {
    Ok(num) => num,
    Err(err) => return Err(std::io::Error::new(std::io::ErrorKind::Other, "read_line failed...n"))
    };

    let str_arc : Arc<String> = Arc::new(is_copy);
    tx.send(str_arc);
    }


    Ok(())
    }









    share|improve this question


























      up vote
      1
      down vote

      favorite









      up vote
      1
      down vote

      favorite











      The purpose of this program is to have one thread (main thread) working on I/O reading lines from a file and feeding it to a pool of worker threads whose job is to perform some processing on each line provided. In this case, the processing is running the String.contains() method.



      Please note that as test input, I used a giant .txt file of English dictionary words found here:



      Also note that you need to specify the file name of the file to be scanned as the sole command line argument to the program. I am mostly concerned with the threading model and multithreading implementation but welcome other feedback as well; new Rust programmer.



      spmc crate ver is "0.2.2" for your cargo.toml



      extern crate spmc;
      use spmc::channel;
      use std::thread;
      use std::io::BufReader;
      use std::fs::File;
      use std::io::BufRead;
      use std::u32::MAX;
      use std::env;
      use std::sync::Arc;

      fn main() -> Result<(), std::io::Error>
      {
      let args: Vec<String> = env::args().collect();
      match args.len()
      {
      2 => {},
      _ => return Err(std::io::Error::new(std::io::ErrorKind::Other, "Incorrect number of args"))
      }
      let filename = &args[1] as &str;
      let f1 = File::open(filename)?;

      let mut br = BufReader::new(f1);

      let mut vecData: Vec<String> = Vec::new();

      let (tx, rx) = spmc::channel();

      let mut handles = Vec::new();
      for n in 0..5 {
      let rx = rx.clone();
      handles.push(thread::spawn(move || {
      loop
      {
      let mut line_to_check: Arc<String> = rx.recv().unwrap();
      if line_to_check.contains("test")
      {
      println!("HIT: {}", line_to_check);
      }
      }

      }));
      }
      let mut input_str = String::new();
      let mut bytes_read: usize = 1;
      while bytes_read != 0 {
      let mut is_copy = input_str.clone();
      bytes_read =
      match br.read_line(&mut is_copy)
      {
      Ok(num) => num,
      Err(err) => return Err(std::io::Error::new(std::io::ErrorKind::Other, "read_line failed...n"))
      };

      let str_arc : Arc<String> = Arc::new(is_copy);
      tx.send(str_arc);
      }


      Ok(())
      }









      share|improve this question















      The purpose of this program is to have one thread (main thread) working on I/O reading lines from a file and feeding it to a pool of worker threads whose job is to perform some processing on each line provided. In this case, the processing is running the String.contains() method.



      Please note that as test input, I used a giant .txt file of English dictionary words found here:



      Also note that you need to specify the file name of the file to be scanned as the sole command line argument to the program. I am mostly concerned with the threading model and multithreading implementation but welcome other feedback as well; new Rust programmer.



      spmc crate ver is "0.2.2" for your cargo.toml



      extern crate spmc;
      use spmc::channel;
      use std::thread;
      use std::io::BufReader;
      use std::fs::File;
      use std::io::BufRead;
      use std::u32::MAX;
      use std::env;
      use std::sync::Arc;

      fn main() -> Result<(), std::io::Error>
      {
      let args: Vec<String> = env::args().collect();
      match args.len()
      {
      2 => {},
      _ => return Err(std::io::Error::new(std::io::ErrorKind::Other, "Incorrect number of args"))
      }
      let filename = &args[1] as &str;
      let f1 = File::open(filename)?;

      let mut br = BufReader::new(f1);

      let mut vecData: Vec<String> = Vec::new();

      let (tx, rx) = spmc::channel();

      let mut handles = Vec::new();
      for n in 0..5 {
      let rx = rx.clone();
      handles.push(thread::spawn(move || {
      loop
      {
      let mut line_to_check: Arc<String> = rx.recv().unwrap();
      if line_to_check.contains("test")
      {
      println!("HIT: {}", line_to_check);
      }
      }

      }));
      }
      let mut input_str = String::new();
      let mut bytes_read: usize = 1;
      while bytes_read != 0 {
      let mut is_copy = input_str.clone();
      bytes_read =
      match br.read_line(&mut is_copy)
      {
      Ok(num) => num,
      Err(err) => return Err(std::io::Error::new(std::io::ErrorKind::Other, "read_line failed...n"))
      };

      let str_arc : Arc<String> = Arc::new(is_copy);
      tx.send(str_arc);
      }


      Ok(())
      }






      multithreading concurrency queue rust






      share|improve this question















      share|improve this question













      share|improve this question




      share|improve this question








      edited yesterday









      200_success

      127k15149412




      127k15149412










      asked Oct 23 at 23:29









      the_endian

      386111




      386111






















          1 Answer
          1






          active

          oldest

          votes

















          up vote
          0
          down vote













          Your code has a number of warnings when compiled under rustc. These are are free code review hints, listen to them!



          You use std::io::Error for all your errors. This is dubious since many of your errors are not I/O related. Personally, I like to use Error from the failure crate. All other errors can be automatically converted it failure::Error by the ? operator.



          A block of code towards the end has a number of issues



          let mut input_str = String::new();
          let mut bytes_read: usize = 1;
          while bytes_read != 0 {
          let mut is_copy = input_str.clone();


          The way your code works, input_str is never modified. As such you are always cloning an empty string for every iteration of the loop. You should just create an empty string here, and get rid of input_Str altogether.



              bytes_read = 
          match br.read_line(&mut is_copy)
          {
          Ok(num) => num,
          Err(err) => return Err(std::io::Error::new(std::io::ErrorKind::Other, "read_line failed...n"))
          };


          The whole point of the read_line function is that you can reuse the memory allocated in String between iterations of your loop. Since you don't do this, this is the wrong interface. Instead use for line in br.lines() to get an iterator over the lines in the file.



              let str_arc : Arc<String> = Arc::new(is_copy);
          tx.send(str_arc);


          There is no reason to use an Arc here. You only need an Arc is both the sending and receiving threads are going to keep a reference to the String. However, that's not the case here. Here you can just send the String from one thread to another, no Arc required.



          }


          On a general note, this is a bad use of threading. Your program is going to be limited by how quickly it can read the data in from the disk. As such, you cannot get a speed boost by multithreading it like this.






          share|improve this answer





















            Your Answer





            StackExchange.ifUsing("editor", function () {
            return StackExchange.using("mathjaxEditing", function () {
            StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
            StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
            });
            });
            }, "mathjax-editing");

            StackExchange.ifUsing("editor", function () {
            StackExchange.using("externalEditor", function () {
            StackExchange.using("snippets", function () {
            StackExchange.snippets.init();
            });
            });
            }, "code-snippets");

            StackExchange.ready(function() {
            var channelOptions = {
            tags: "".split(" "),
            id: "196"
            };
            initTagRenderer("".split(" "), "".split(" "), channelOptions);

            StackExchange.using("externalEditor", function() {
            // Have to fire editor after snippets, if snippets enabled
            if (StackExchange.settings.snippets.snippetsEnabled) {
            StackExchange.using("snippets", function() {
            createEditor();
            });
            }
            else {
            createEditor();
            }
            });

            function createEditor() {
            StackExchange.prepareEditor({
            heartbeatType: 'answer',
            convertImagesToLinks: false,
            noModals: true,
            showLowRepImageUploadWarning: true,
            reputationToPostImages: null,
            bindNavPrevention: true,
            postfix: "",
            imageUploader: {
            brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
            contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
            allowUrls: true
            },
            onDemand: true,
            discardSelector: ".discard-answer"
            ,immediatelyShowMarkdownHelp:true
            });


            }
            });














            draft saved

            draft discarded


















            StackExchange.ready(
            function () {
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f206142%2frust-task-queue%23new-answer', 'question_page');
            }
            );

            Post as a guest















            Required, but never shown

























            1 Answer
            1






            active

            oldest

            votes








            1 Answer
            1






            active

            oldest

            votes









            active

            oldest

            votes






            active

            oldest

            votes








            up vote
            0
            down vote













            Your code has a number of warnings when compiled under rustc. These are are free code review hints, listen to them!



            You use std::io::Error for all your errors. This is dubious since many of your errors are not I/O related. Personally, I like to use Error from the failure crate. All other errors can be automatically converted it failure::Error by the ? operator.



            A block of code towards the end has a number of issues



            let mut input_str = String::new();
            let mut bytes_read: usize = 1;
            while bytes_read != 0 {
            let mut is_copy = input_str.clone();


            The way your code works, input_str is never modified. As such you are always cloning an empty string for every iteration of the loop. You should just create an empty string here, and get rid of input_Str altogether.



                bytes_read = 
            match br.read_line(&mut is_copy)
            {
            Ok(num) => num,
            Err(err) => return Err(std::io::Error::new(std::io::ErrorKind::Other, "read_line failed...n"))
            };


            The whole point of the read_line function is that you can reuse the memory allocated in String between iterations of your loop. Since you don't do this, this is the wrong interface. Instead use for line in br.lines() to get an iterator over the lines in the file.



                let str_arc : Arc<String> = Arc::new(is_copy);
            tx.send(str_arc);


            There is no reason to use an Arc here. You only need an Arc is both the sending and receiving threads are going to keep a reference to the String. However, that's not the case here. Here you can just send the String from one thread to another, no Arc required.



            }


            On a general note, this is a bad use of threading. Your program is going to be limited by how quickly it can read the data in from the disk. As such, you cannot get a speed boost by multithreading it like this.






            share|improve this answer

























              up vote
              0
              down vote













              Your code has a number of warnings when compiled under rustc. These are are free code review hints, listen to them!



              You use std::io::Error for all your errors. This is dubious since many of your errors are not I/O related. Personally, I like to use Error from the failure crate. All other errors can be automatically converted it failure::Error by the ? operator.



              A block of code towards the end has a number of issues



              let mut input_str = String::new();
              let mut bytes_read: usize = 1;
              while bytes_read != 0 {
              let mut is_copy = input_str.clone();


              The way your code works, input_str is never modified. As such you are always cloning an empty string for every iteration of the loop. You should just create an empty string here, and get rid of input_Str altogether.



                  bytes_read = 
              match br.read_line(&mut is_copy)
              {
              Ok(num) => num,
              Err(err) => return Err(std::io::Error::new(std::io::ErrorKind::Other, "read_line failed...n"))
              };


              The whole point of the read_line function is that you can reuse the memory allocated in String between iterations of your loop. Since you don't do this, this is the wrong interface. Instead use for line in br.lines() to get an iterator over the lines in the file.



                  let str_arc : Arc<String> = Arc::new(is_copy);
              tx.send(str_arc);


              There is no reason to use an Arc here. You only need an Arc is both the sending and receiving threads are going to keep a reference to the String. However, that's not the case here. Here you can just send the String from one thread to another, no Arc required.



              }


              On a general note, this is a bad use of threading. Your program is going to be limited by how quickly it can read the data in from the disk. As such, you cannot get a speed boost by multithreading it like this.






              share|improve this answer























                up vote
                0
                down vote










                up vote
                0
                down vote









                Your code has a number of warnings when compiled under rustc. These are are free code review hints, listen to them!



                You use std::io::Error for all your errors. This is dubious since many of your errors are not I/O related. Personally, I like to use Error from the failure crate. All other errors can be automatically converted it failure::Error by the ? operator.



                A block of code towards the end has a number of issues



                let mut input_str = String::new();
                let mut bytes_read: usize = 1;
                while bytes_read != 0 {
                let mut is_copy = input_str.clone();


                The way your code works, input_str is never modified. As such you are always cloning an empty string for every iteration of the loop. You should just create an empty string here, and get rid of input_Str altogether.



                    bytes_read = 
                match br.read_line(&mut is_copy)
                {
                Ok(num) => num,
                Err(err) => return Err(std::io::Error::new(std::io::ErrorKind::Other, "read_line failed...n"))
                };


                The whole point of the read_line function is that you can reuse the memory allocated in String between iterations of your loop. Since you don't do this, this is the wrong interface. Instead use for line in br.lines() to get an iterator over the lines in the file.



                    let str_arc : Arc<String> = Arc::new(is_copy);
                tx.send(str_arc);


                There is no reason to use an Arc here. You only need an Arc is both the sending and receiving threads are going to keep a reference to the String. However, that's not the case here. Here you can just send the String from one thread to another, no Arc required.



                }


                On a general note, this is a bad use of threading. Your program is going to be limited by how quickly it can read the data in from the disk. As such, you cannot get a speed boost by multithreading it like this.






                share|improve this answer












                Your code has a number of warnings when compiled under rustc. These are are free code review hints, listen to them!



                You use std::io::Error for all your errors. This is dubious since many of your errors are not I/O related. Personally, I like to use Error from the failure crate. All other errors can be automatically converted it failure::Error by the ? operator.



                A block of code towards the end has a number of issues



                let mut input_str = String::new();
                let mut bytes_read: usize = 1;
                while bytes_read != 0 {
                let mut is_copy = input_str.clone();


                The way your code works, input_str is never modified. As such you are always cloning an empty string for every iteration of the loop. You should just create an empty string here, and get rid of input_Str altogether.



                    bytes_read = 
                match br.read_line(&mut is_copy)
                {
                Ok(num) => num,
                Err(err) => return Err(std::io::Error::new(std::io::ErrorKind::Other, "read_line failed...n"))
                };


                The whole point of the read_line function is that you can reuse the memory allocated in String between iterations of your loop. Since you don't do this, this is the wrong interface. Instead use for line in br.lines() to get an iterator over the lines in the file.



                    let str_arc : Arc<String> = Arc::new(is_copy);
                tx.send(str_arc);


                There is no reason to use an Arc here. You only need an Arc is both the sending and receiving threads are going to keep a reference to the String. However, that's not the case here. Here you can just send the String from one thread to another, no Arc required.



                }


                On a general note, this is a bad use of threading. Your program is going to be limited by how quickly it can read the data in from the disk. As such, you cannot get a speed boost by multithreading it like this.







                share|improve this answer












                share|improve this answer



                share|improve this answer










                answered Nov 7 at 19:03









                Winston Ewert

                26.5k44474




                26.5k44474






























                    draft saved

                    draft discarded




















































                    Thanks for contributing an answer to Code Review Stack Exchange!


                    • Please be sure to answer the question. Provide details and share your research!

                    But avoid



                    • Asking for help, clarification, or responding to other answers.

                    • Making statements based on opinion; back them up with references or personal experience.


                    Use MathJax to format equations. MathJax reference.


                    To learn more, see our tips on writing great answers.





                    Some of your past answers have not been well-received, and you're in danger of being blocked from answering.


                    Please pay close attention to the following guidance:


                    • Please be sure to answer the question. Provide details and share your research!

                    But avoid



                    • Asking for help, clarification, or responding to other answers.

                    • Making statements based on opinion; back them up with references or personal experience.


                    To learn more, see our tips on writing great answers.




                    draft saved


                    draft discarded














                    StackExchange.ready(
                    function () {
                    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f206142%2frust-task-queue%23new-answer', 'question_page');
                    }
                    );

                    Post as a guest















                    Required, but never shown





















































                    Required, but never shown














                    Required, but never shown












                    Required, but never shown







                    Required, but never shown

































                    Required, but never shown














                    Required, but never shown












                    Required, but never shown







                    Required, but never shown







                    Popular posts from this blog

                    Ellipse (mathématiques)

                    Quarter-circle Tiles

                    Mont Emei