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

Dev: move fn mg_timer_add #170

Merged
merged 19 commits into from
Jul 22, 2023
Merged

Dev: move fn mg_timer_add #170

merged 19 commits into from
Jul 22, 2023

Conversation

lanytcc
Copy link
Contributor

@lanytcc lanytcc commented Jul 22, 2023

  1. 我搜了一下文档,mg_timer_add函数调用一次即可,放置在循环里可能会导致严重的问题
  2. 顺便优化了一下onMessage函数调用带来的内存和cpu开销

@xfangfang
Copy link
Owner

感谢,这个位置我确实忽视了。

@xfangfang
Copy link
Owner

mg_timer_free 是不是不需要调用,在mongoose_thread结束时调用了 mg_mgr_free, 这个函数内部会直接free掉所有timer。
在 mongoose_thread join 之后貌似就不应该调用 mg_timer_free 了。

@xfangfang
Copy link
Owner

有点好奇,你是没有本地环境直接在网页上改的程序嘛?

@lanytcc
Copy link
Contributor Author

lanytcc commented Jul 22, 2023

虽然不是,但也差不多了(
环境有问题,最多用docker。但一样生成不了本地的compiled_commands
image

@xfangfang
Copy link
Owner

哈哈,原来如此。
不确定这个软件能不能直接用 vs 编译,我之前是在 MSYS2 的 MinGW64 环境中编译的,README中有写相关的内容,感觉还是得本地有个环境,不然太容易出问题了。

@zeromake
Copy link
Collaborator

哈哈,原来如此。
不确定这个软件能不能直接用 vs 编译,我之前是在 MSYS2 的 MinGW64 环境中编译的,README中有写相关的内容,感觉还是得本地有个环境,不然太容易出问题了。

……当然是可以用msvc的,当初做uwp的时候就兼容了,我本地都是用xmake + vscode的,xmake导出vs工程应该也可以直接用vs开发。

@lanytcc
Copy link
Contributor Author

lanytcc commented Jul 22, 2023

@zeromake 提示包yoga、tweeny等找不到,我该怎么办?

@lanytcc
Copy link
Contributor Author

lanytcc commented Jul 22, 2023

@xfangfang 可以合并了,我这边测试没有问题,cpu占用跟正常看视频没有区别了(甚至更低),我给danmaku_core 增加了两个函数,你看一下是否合理,其他都是删减,接口没有发生变化

uint32_t packet_length = ntohl(*reinterpret_cast<const uint32_t*>(data.data() + offset));
uint16_t header_length = ntohs(*reinterpret_cast<const uint16_t*>(data.data() + offset + 4));
uint16_t protocol_version = ntohs(*reinterpret_cast<const uint16_t*>(data.data() + offset + 6));
uint32_t operation = ntohl(*reinterpret_cast<const uint32_t*>(data.data() + offset + 8));
Copy link
Owner

Choose a reason for hiding this comment

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

这一段可以找到更好的方式来实现吗?直接使用 reinterpret_cast 转换两个数字类型会导致 Undefined Behavior,虽然大多数时候都能正常运行,但是还是应该避免这个警告。

https://stackoverflow.com/a/25586060

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我使用的uint32_t应该能在大部分平台保持一致吧,你引用的问题使用了reinterpret_cast<unsigned long>(int),他这个确实可能会有问题
要不就换回我之前使用的memcpy。。

Copy link
Owner

Choose a reason for hiding this comment

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

@maye174 你之前就是写的 reinterpret_cast,这里的 memcpy 是我改的,如果不改的话在调试的时候IDE一直报告问题。
我在这里有提及:#164 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xfangfang 那你再改回去吧,我真没注意这个,我比较惯用这个。。

borderColor.a = DanmakuCore::DANMAKU_STYLE_ALPHA * 0.5;
}
}

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
Contributor Author

Choose a reason for hiding this comment

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

那传递的时候依旧需要复制一遍,就没有意义了

Copy link
Contributor Author

Choose a reason for hiding this comment

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

如果你指的是单独拿出重复的部分,那是没有问题的

Copy link
Owner

Choose a reason for hiding this comment

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

如果你指的是单独拿出重复的部分,那是没有问题的

是的我就是这个意思,这样之后改的话也不需要同时改两个地方

@xfangfang
Copy link
Owner

@maye174 那我先合并了自己改一下,还是你把那两处改一下呢?

@xfangfang xfangfang merged commit e8a29f9 into xfangfang:dev Jul 22, 2023
11 checks passed
@lanytcc
Copy link
Contributor Author

lanytcc commented Jul 22, 2023

@maye174 那我先合并了自己改一下,还是你把那两处改一下呢?

等会我来改

@xfangfang
Copy link
Owner

@maye174 那我先合并了自己改一下,还是你把那两处改一下呢?

等会我来改

reinterpret_cast 那个问题我刚刚测试又可以正常用了,可能是其他原因导致的之前的版本那里有问题。所以那里先不用改了~

万分感谢,我再去搞一搞flatpak构建的时候缓存总是缓存不到的问题,这些搞好没啥问题的话,咱就发个新版本,今天真的辛苦啦~

@zeromake
Copy link
Collaborator

@zeromake 提示包yoga、tweeny等找不到,我该怎么办?

先注意一下 xmake xrepo 是否添加了 zeromake/xrepo 的源。
如果添加了还没有,就得看看 xmake f -c -vD -y 的具体输出了。

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