Ptt-official-app / go-bbs

BBS file database manager system library in Golang.
Apache License 2.0
80 stars 43 forks source link

add user information struct #63

Closed kiraxie closed 3 years ago

kiraxie commented 3 years ago

👏 解決掉的 issue / Resolved Issues

📝 相關的 issue / Related Issues

⛏ 變更內容 / Details of Changes

PichuChen commented 3 years ago

看能不能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 commented 3 years ago

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

kiraxie commented 3 years ago

看能不能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 問題,最後結論是啥呢?

PichuChen commented 3 years ago

看能不能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 開發慣例上的形式就行了