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

增加性能数据方法 #37

Merged
merged 27 commits into from
Aug 9, 2022
Merged

增加性能数据方法 #37

merged 27 commits into from
Aug 9, 2022

Conversation

aoliaoaoaojiao
Copy link

1.新增CPU、内存、GPU、FPS、系统网络数据获取方法
2.新增停止方法
3.增加性能数据测试方法

根据tidevice通信移植,由于未系统学习过Go,细节可能处理得不是很好

@ZhouYixun
Copy link
Contributor

好耶~ 刚好今天开头整这个,就看到你提pr啦~

@aoliaoaoaojiao
Copy link
Author

好耶今天刚看完这个,就看到你提pr啦

sonic-ios-bridge的部分其实我也写了,等合并通过了再改改go.mod就能提交

@ZhouYixun
Copy link
Contributor

ZhouYixun commented Jul 18, 2022

好耶今天刚看完这个,就看到你提pr啦

sonic-ios-bridge的部分其实我也写了,等合并通过了再改改go.mod就能提交

有看到啦,加微信讨论下不?

@aoliaoaoaojiao
Copy link
Author

好耶今天刚看完这个,就看到你提pr啦

sonic-ios-bridge的部分其实我也写了,等合并通过了再改改go.mod就能提交

有看到啦,加微信讨论下不?

啊啊,可以的,请问如何联系?

@ZhouYixun
Copy link
Contributor

ZhouYixun commented Jul 18, 2022

@alliaoaoo 我vx是*********

@electricbubble
Copy link
Owner

最近有点忙,这个 PR 可能要晚一点处理

Copy link
Owner

@electricbubble electricbubble left a comment

Choose a reason for hiding this comment

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

整体结构上还需要做一些调整,

如果对此还感兴趣的话,可以等你调整后再合并

device.go Outdated Show resolved Hide resolved
device.go Outdated Show resolved Hide resolved
device.go Outdated Show resolved Hide resolved
@ZhouYixun
Copy link
Contributor

@aoliaoaoaojiao 怎么你改strust之后,你json:后面不是驼峰命名的,而且还包含空格,是不是不太规范

@aoliaoaoaojiao
Copy link
Author

@electricbubble 已优化,大佬过一下

device.go Outdated
Comment on lines 601 to 603
if len(opts) == 0 {
perfmonOpts = nil
return nil, nil, fmt.Errorf("parameter error,please enter relevant parameters")
Copy link
Owner

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.

从本质上这个是用户通过参数自行决定开启性能通道的方法,没有需要必填的性能参数项,所以这个len(ops)的判断按我理解直接去掉,就行了,用户如果不输入性能直接走下面的参数flag判断就行了

Copy link
Author

Choose a reason for hiding this comment

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

或者直接去掉GetPerfmon相关方法,因为这个方法本质上是应用层上的事情,获取性能让用户直接用Instruments,这样的话更符合通信层的定义

Copy link
Author

Choose a reason for hiding this comment

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

@electricbubble 希望大佬回复下是否保留device的GetPerfmon相关方法

Copy link
Owner

@electricbubble electricbubble Jul 31, 2022

Choose a reason for hiding this comment

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

这个函数的定位和 XCTest 是一致的,用户如果愿意,可以通过使用 pkg 自行实现,

但特意拎出来实现,就是为了简化普通用户的使用,

同理,GetPerfmon 也需要在使用上足够简单。


按照现在对 Options 的逻辑判断,这是一个 必填参数

如果现在用户使用,默认情况,极大概率会选择不传入这个参数,那就会直接报错,

在得到报错后再 “被迫” 传入参数,

不如直接取消这个选项模式,直接用 struct 代替,

如果是选项模式,也应当提供一个在用户不传参数时,默认全部开启的配置

在用户使用时,就能自己决定是否开启想要的功能。


同时,现在的 PerfmonOption 使用的默认 false 配合 ! 认为是开启?

Copy link
Author

Choose a reason for hiding this comment

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

这个函数的定位和 XCTest 是一致的,用户如果愿意,可以通过使用 pkg 自行实现,

但特意拎出来实现,就是为了简化普通用户的使用,

同理,GetPerfmon 也需要在使用上足够简单。

按照现在对 Options 的逻辑判断,这是一个 必填参数

如果现在用户使用,默认情况,极大概率会选择不传入这个参数,那就会直接报错,

在得到报错后再 “被迫” 传入参数,

不如直接取消这个选项模式,直接用 struct 代替,

如果是选项模式,也应当提供一个在用户不传参数时,默认全部开启的配置

在用户使用时,就能自己决定是否开启想要的功能。

同时,现在的 PerfmonOption 使用的默认 false 配合 ! 认为是开启?

按这么理解的话,大佬你看这么改合适否:
1.性能参数单独定义一个struct,使用时&PerfmonOption{ paramField : paramValue } 传进去,如果方法里判断到PerfmonOption未定义或者里面的性能flag全为false,则返回默认的性能数据
2.Instruments的相关方法暴露出来,让用户决定是否自行实现

Copy link
Author

Choose a reason for hiding this comment

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

@electricbubble 修改已提交,大佬看看这么改是否合适

pkg/performance/cpuInfo.go Outdated Show resolved Hide resolved
pkg/performance/cpuInfo.go Outdated Show resolved Hide resolved
pkg/performance/perfmonData.go Outdated Show resolved Hide resolved
device.go Outdated Show resolved Hide resolved
device.go Outdated Show resolved Hide resolved
pkg/performance/cpuInfo.go Outdated Show resolved Hide resolved
……son、ToFormat方法

2.隐藏StopPerfmon方法,调用GetPerfmon返回的context.CancelFunc直接关闭
3.弃用PerfMonData通道处理数据,原始性能数据统一输送到interface通道里,由用户自行决定数据的处理
4.CPU、MEM优化异常PID信息提示
5.GetPerfmon参数优化,默认展示所有性能数据
6.fix cpu、mem plist Unknown parameter
@ZhouYixun
Copy link
Contributor

@electricbubble 大佬再看看

@electricbubble
Copy link
Owner

electricbubble commented Aug 8, 2022

等有空了我直接在你们的分支上修改, 有太多不符合我个人预期的改动

@ZhouYixun
Copy link
Contributor

等有空了我直接在你们的分支上修改, 有太多不符合我个人预期的改动

明白,感谢

@ZhouYixun
Copy link
Contributor

等有空了我直接在你们的分支上修改, 有太多不符合我个人预期的改动

泡泡哥,你要不开个dev分支,我们先提给dev,你也可以参与改动了。因为后面我们还有webinspector,可能都是代码量比较多,你在我们分支改可能比较麻烦

@electricbubble electricbubble changed the base branch from main to dev-performance-monitoring August 9, 2022 14:25
@electricbubble electricbubble merged commit 8bef4cc into electricbubble:dev-performance-monitoring Aug 9, 2022
@electricbubble
Copy link
Owner

🉑️

暂时放 dev-performance-monitoring 了,

最近要处理的事情确实多,我尽可能腾出时间参与修改

@ZhouYixun
Copy link
Contributor

🉑️

暂时放 dev-performance-monitoring 了,

最近要处理的事情确实多,我尽可能腾出时间参与修改

好的

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.

3 participants