Skip to main content
replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

There are two things I see.

  • Don't store a reference to a time out. I think the count down starts when After() is called.

      timeout := time.After(2000 * time.Millisecond)
      ...
      case <- timeout:
    

    Most code I've seen writes it like this.

      case <- time.After(2000 * time.Millisecond):
    
  • Use a lock when accessing a map from multiple goroutines because maps aren't thread safe.

Better solution:

func Crawl(url string, depth int, fetcher Fetcher) {
    m := map[string]bool{url: true}
    var mx sync.Mutex
    var wg sync.WaitGroup
    var c2 func(string, int)
    c2 = func(url string, depth int) {
        defer wg.Done()
        if depth <= 0 {
            return
        }
        body, urls, err := fetcher.Fetch(url)
        if err != nil {
            fmt.Println(err)
            return
        }
        fmt.Printf("found: %s %q\n", url, body)
        mx.Lock()
        for _, u := range urls {
            if !m[u] {
                m[u] = true
                wg.Add(1)
                go c2(u, depth-1)
            }
        }
        mx.Unlock()
    }
    wg.Add(1)
    c2(url, depth)
    wg.Wait()
}

Useful link:

http://stackoverflow.com/questions/12224962/exercise-web-crawler-concurrency-not-workinghttps://stackoverflow.com/questions/12224962/exercise-web-crawler-concurrency-not-working

There are two things I see.

  • Don't store a reference to a time out. I think the count down starts when After() is called.

      timeout := time.After(2000 * time.Millisecond)
      ...
      case <- timeout:
    

    Most code I've seen writes it like this.

      case <- time.After(2000 * time.Millisecond):
    
  • Use a lock when accessing a map from multiple goroutines because maps aren't thread safe.

Better solution:

func Crawl(url string, depth int, fetcher Fetcher) {
    m := map[string]bool{url: true}
    var mx sync.Mutex
    var wg sync.WaitGroup
    var c2 func(string, int)
    c2 = func(url string, depth int) {
        defer wg.Done()
        if depth <= 0 {
            return
        }
        body, urls, err := fetcher.Fetch(url)
        if err != nil {
            fmt.Println(err)
            return
        }
        fmt.Printf("found: %s %q\n", url, body)
        mx.Lock()
        for _, u := range urls {
            if !m[u] {
                m[u] = true
                wg.Add(1)
                go c2(u, depth-1)
            }
        }
        mx.Unlock()
    }
    wg.Add(1)
    c2(url, depth)
    wg.Wait()
}

Useful link:

http://stackoverflow.com/questions/12224962/exercise-web-crawler-concurrency-not-working

There are two things I see.

  • Don't store a reference to a time out. I think the count down starts when After() is called.

      timeout := time.After(2000 * time.Millisecond)
      ...
      case <- timeout:
    

    Most code I've seen writes it like this.

      case <- time.After(2000 * time.Millisecond):
    
  • Use a lock when accessing a map from multiple goroutines because maps aren't thread safe.

Better solution:

func Crawl(url string, depth int, fetcher Fetcher) {
    m := map[string]bool{url: true}
    var mx sync.Mutex
    var wg sync.WaitGroup
    var c2 func(string, int)
    c2 = func(url string, depth int) {
        defer wg.Done()
        if depth <= 0 {
            return
        }
        body, urls, err := fetcher.Fetch(url)
        if err != nil {
            fmt.Println(err)
            return
        }
        fmt.Printf("found: %s %q\n", url, body)
        mx.Lock()
        for _, u := range urls {
            if !m[u] {
                m[u] = true
                wg.Add(1)
                go c2(u, depth-1)
            }
        }
        mx.Unlock()
    }
    wg.Add(1)
    c2(url, depth)
    wg.Wait()
}

Useful link:

https://stackoverflow.com/questions/12224962/exercise-web-crawler-concurrency-not-working

Source Link
Larry Battle
  • 2.2k
  • 11
  • 19

There are two things I see.

  • Don't store a reference to a time out. I think the count down starts when After() is called.

      timeout := time.After(2000 * time.Millisecond)
      ...
      case <- timeout:
    

    Most code I've seen writes it like this.

      case <- time.After(2000 * time.Millisecond):
    
  • Use a lock when accessing a map from multiple goroutines because maps aren't thread safe.

Better solution:

func Crawl(url string, depth int, fetcher Fetcher) {
    m := map[string]bool{url: true}
    var mx sync.Mutex
    var wg sync.WaitGroup
    var c2 func(string, int)
    c2 = func(url string, depth int) {
        defer wg.Done()
        if depth <= 0 {
            return
        }
        body, urls, err := fetcher.Fetch(url)
        if err != nil {
            fmt.Println(err)
            return
        }
        fmt.Printf("found: %s %q\n", url, body)
        mx.Lock()
        for _, u := range urls {
            if !m[u] {
                m[u] = true
                wg.Add(1)
                go c2(u, depth-1)
            }
        }
        mx.Unlock()
    }
    wg.Add(1)
    c2(url, depth)
    wg.Wait()
}

Useful link:

http://stackoverflow.com/questions/12224962/exercise-web-crawler-concurrency-not-working