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

add user information struct #63

Closed
wants to merge 1 commit into from
Closed

add user information struct #63

wants to merge 1 commit into from

Conversation

kiraxie
Copy link

@kiraxie kiraxie commented Mar 6, 2021

👏 解決掉的 issue / Resolved Issues

📝 相關的 issue / Related Issues

  • None

⛏ 變更內容 / Details of Changes

  • 新增 user information 相關 struct (REF)


const (
userDataSize int64 = 80
friendShipSize int64 = 2196
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

這個這樣一般人應該很難回答這個數字是從哪邊來的?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

這個我拿掉,改用註解方式

userInfo.go Show resolved Hide resolved
userInfo.go Show resolved Hide resolved
userInfo.go Show resolved Hide resolved
WbTime int64
}

func (t *UserInfo) ReadFrom(r io.Reader) (_ int64, err error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shared memory 如果用 ReadForm 的形式作操作的話,可能會比較慢?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

對,但是穩定性無法比擬

回到這個案子的最原始的問題,你想要解決效能問題,還是穩定問題?

Copy link
Collaborator

@titaneric titaneric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

看能不能follow類似pttbbs/passwd.go的寫法,這樣struct之間寫法才比較一致。

  • 定義開始位置
  • 定義string長度常數,例如PTT_CAREERSZ
  • padding部份一樣另外加

@titaneric titaneric self-requested a review March 6, 2021 09:00
Copy link
Collaborator

@titaneric titaneric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

看能不能follow類似pttbbs/passwd.go的寫法,這樣struct之間寫法才比較一致。

  • 定義開始位置
  • 定義string長度常數,例如PTT_CAREERSZ
  • padding部份一樣另外加

@PichuChen
Copy link
Member

看能不能follow類似pttbbs/passwd.go的寫法,這樣struct之間寫法才比較一致。

  • 定義開始位置
  • 定義string長度常數,例如PTT_CAREERSZ
  • padding部份一樣另外加

原則上是希望能夠 follow passwd.go ,但是發現有些技術上的問題,所以讓事情變得複雜一點點。

因為這邊幾個 struct 他是操作 Shared Memory, 所以在操作上有可能要避免不必要的複製,在寫入時也要避免不必要的寫入。

舉例來說,如果一個結構體有 a b c 三個欄位時,如果我們只有要讀取 b 那就只讀取 b 而避免 a b c 三個都讀取
如果是寫入 c 的話那也是避免同時寫入 a b

在 passwd.go 裡面的話,我們如果只要知道某個使用者的資訊,我們會把這個使用者的資訊完整讀進來之後使用某個欄位
寫入時也是把整個使用者寫回去。

但是在 Shared Memory 的話這樣會讓效能和原本的做法比起來會慢很多(實際上差異要跑一下Benchmark),另外因為同一塊記憶體空間會由多個Process同時存取的關係,也會增加不同步的風險,當然不同步的風險應該是要用 Lock 去處理,但是大面積的讀寫同時也會增加Lock所需要處理的範圍。

@titaneric
Copy link
Collaborator

抱歉,第一次review有點弄亂版面,請忽略重複訊息

@kiraxie kiraxie closed this Mar 14, 2021
@kiraxie
Copy link
Author

kiraxie commented Mar 14, 2021

看能不能follow類似pttbbs/passwd.go的寫法,這樣struct之間寫法才比較一致。

  • 定義開始位置
  • 定義string長度常數,例如PTT_CAREERSZ
  • padding部份一樣另外加

原則上是希望能夠 follow passwd.go ,但是發現有些技術上的問題,所以讓事情變得複雜一點點。

因為這邊幾個 struct 他是操作 Shared Memory, 所以在操作上有可能要避免不必要的複製,在寫入時也要避免不必要的寫入。

舉例來說,如果一個結構體有 a b c 三個欄位時,如果我們只有要讀取 b 那就只讀取 b 而避免 a b c 三個都讀取
如果是寫入 c 的話那也是避免同時寫入 a b

在 passwd.go 裡面的話,我們如果只要知道某個使用者的資訊,我們會把這個使用者的資訊完整讀進來之後使用某個欄位
寫入時也是把整個使用者寫回去。

但是在 Shared Memory 的話這樣會讓效能和原本的做法比起來會慢很多(實際上差異要跑一下Benchmark),另外因為同一塊記憶體空間會由多個Process同時存取的關係,也會增加不同步的風險,當然不同步的風險應該是要用 Lock 去處理,但是大面積的讀寫同時也會增加Lock所需要處理的範圍。

今天稍微改變了作法,做成同種做法不是問題,但我認為這樣直接操作 raw slice 風險其實很高,因為完全是依靠人為計算offset,這樣做法並沒有一些保護,所以存在 memory leak 的風險,如果要做到相對應的保護的話,其實不是很好用,大致上如下所示

type userData struct {
	buff       []byte
}
func (t *userData) Level() (uint32, error) {
	var b [4]byte
	if _, err := bytes.NewReader(t.buff).ReadAt(b[:], PosOfPttUserDataLevel); err != nil {
		return 0, err
	}

	return binary.LittleEndian.Uint32(b[:])
}

當然可以很簡單的

type userData struct {
	buff       []byte
}
func (t *userData) Level() (uint32, error) {
	return binary.LittleEndian.Uint32(t.buff[ : PosOfPttUserDataLevel+4])
}

但我想兩者安全性跟穩定性不在同一個標準上

這個部分,如果單純追求所謂的效能的確後者簡單粗暴,但我覺得我看到的風險問題不是可以忽略的,大家覺得呢?

另外 @PichuChen 上次提到的 uint test package 問題,最後結論是啥呢?

@kiraxie kiraxie reopened this Mar 14, 2021
@PichuChen
Copy link
Member

看能不能follow類似pttbbs/passwd.go的寫法,這樣struct之間寫法才比較一致。

  • 定義開始位置
  • 定義string長度常數,例如PTT_CAREERSZ
  • padding部份一樣另外加

原則上是希望能夠 follow passwd.go ,但是發現有些技術上的問題,所以讓事情變得複雜一點點。
因為這邊幾個 struct 他是操作 Shared Memory, 所以在操作上有可能要避免不必要的複製,在寫入時也要避免不必要的寫入。
舉例來說,如果一個結構體有 a b c 三個欄位時,如果我們只有要讀取 b 那就只讀取 b 而避免 a b c 三個都讀取
如果是寫入 c 的話那也是避免同時寫入 a b
在 passwd.go 裡面的話,我們如果只要知道某個使用者的資訊,我們會把這個使用者的資訊完整讀進來之後使用某個欄位
寫入時也是把整個使用者寫回去。
但是在 Shared Memory 的話這樣會讓效能和原本的做法比起來會慢很多(實際上差異要跑一下Benchmark),另外因為同一塊記憶體空間會由多個Process同時存取的關係,也會增加不同步的風險,當然不同步的風險應該是要用 Lock 去處理,但是大面積的讀寫同時也會增加Lock所需要處理的範圍。

今天稍微改變了作法,做成同種做法不是問題,但我認為這樣直接操作 raw slice 風險其實很高,因為完全是依靠人為計算offset,這樣做法並沒有一些保護,所以存在 memory leak 的風險,如果要做到相對應的保護的話,其實不是很好用,大致上如下所示

因為有做 GC 基本上應該是不會 memory leak, 當然比較好的作法應該再補一下testing

type userData struct {
	buff       []byte
}
func (t *userData) Level() (uint32, error) {
	var b [4]byte
	if _, err := bytes.NewReader(t.buff).ReadAt(b[:], PosOfPttUserDataLevel); err != nil {
		return 0, err
	}

	return binary.LittleEndian.Uint32(b[:])
}

當然可以很簡單的

type userData struct {
	buff       []byte
}
func (t *userData) Level() (uint32, error) {
	return binary.LittleEndian.Uint32(t.buff[ : PosOfPttUserDataLevel+4])
}

但我想兩者安全性跟穩定性不在同一個標準上

這個部分,如果單純追求所謂的效能的確後者簡單粗暴,但我覺得我看到的風險問題不是可以忽略的,大家覺得呢?

這部分的風險應該透過 testing code 可以加以管理

另外 @PichuChen 上次提到的 uint test package 問題,最後結論是啥呢?

這問題應該還是保留原本 Go 開發慣例上的形式就行了

@PichuChen PichuChen closed this Mar 20, 2021
@PichuChen PichuChen deleted the branch Ptt-official-app:feature/cache March 20, 2021 06:42
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.

None yet

3 participants