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

fix duration of mosaic conversion #17

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

MohammadAlavi1986
Copy link

t1 (end time) must be assigned to after we have received converted image from the channel

	t1 := time.Now()
	images := map[string]string{
		"original": originalStr,
		"mosaic":   <- c,
		"duration": fmt.Sprintf("%v ", t1.Sub(t0)),
	}

mus change to:

        mosaicStr := <- c
	t1 := time.Now()
	images := map[string]string{
		"original": originalStr,
		"mosaic":   mosaicStr,
		"duration": fmt.Sprintf("%v ", t1.Sub(t0)),
	}

`t1` (end time) must be assigned to after we have received converted image from the channel
@hyzgh
Copy link

hyzgh commented Jun 19, 2020

I encounter this problem too. The fixed version works correctly. But I couldn't figure out the reason. Do you know anything about the reason?

@MohammadAlavi1986
Copy link
Author

@hyzgh when the channel is read if the channel is not ready the receiver will be blocked until the channel is ready that's the reason why we should assign time.Now() to t1 only after we have read the channel content successfully but the original version assigns to t1 before it reads the channel.

@hyzgh
Copy link

hyzgh commented Jun 25, 2020

@AureaMohammadAlavi
If we read the channel when we initial the map, it will be blocked too. I have written a test to prove it.

package main

import (
	"fmt"
	"time"
)

func getSlow() chan int {
	d := make(chan int)
	go func() {
		time.Sleep(1*time.Second)
		d <- 42
	}()
	return d
}

func main() {
	t0 := time.Now()
	m := map[string]interface{}{
		"mosaic": <-getSlow(),
		"duration": time.Now().Sub(t0),
	}
	fmt.Println(m)
}
// output: map[duration:1.000127088s mosaic:42]

@MohammadAlavi1986
Copy link
Author

MohammadAlavi1986 commented Jun 25, 2020

@hyzgh Yes it will be blocked and that's why we should only calculate the duration after we've read from the channel but in the author's code the duration is calculated (now is assigned to t1) before reading from the channel.

	t1 := time.Now()
	images := map[string]string{
		"original": originalStr,
		"mosaic":   <- c,
		"duration": fmt.Sprintf("%v ", t1.Sub(t0)),
	}

In your sample code you calculate duration after reading from the channel and that's correct.

@hyzgh
Copy link

hyzgh commented Jun 26, 2020 via email

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.

2 participants