Skip to content

feat: 新增 EraseSwap 方法,优化删除性能至 O(1) 复杂度#501

Open
firewiki wants to merge 42 commits into
masterfrom
EraseSwap
Open

feat: 新增 EraseSwap 方法,优化删除性能至 O(1) 复杂度#501
firewiki wants to merge 42 commits into
masterfrom
EraseSwap

Conversation

@firewiki
Copy link
Copy Markdown
Collaborator

No description provided.

Comment thread llbc/src/core/objbase/Array.cpp Outdated
return Erase(Begin() + n0, Begin() + n1);
}

bool LLBC_Array::EraseSwap(LLBC_Array::Obj* o, bool releaseObj)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

建议统一风格:
LLBC_Array::Obj* o -> LLBC_Array::Obj *o

Comment thread llbc/src/core/objbase/Array.cpp Outdated

bool LLBC_Array::EraseSwap(LLBC_Array::Obj* o, bool releaseObj)
{
if (UNLIKELY(o == nullptr)) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

建议统一风格:{ 单独一行

Comment thread llbc/src/core/objbase/Array.cpp Outdated

for (difference_type i = 0; i < _size; ++i)
{
if (_objs[i] == o) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

建议统一风格:{ 单独一行

// Note: EraseSwap does not preserve element order, which is acceptable for AutoReleasePool
if (!_arr->EraseSwap(o, false))
{
LLBC_SetLastError(LLBC_ERROR_NOT_FOUND);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

这里待讨论,是不是EraseSwap返回值是int,这里就不需要重复setLastError了

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

保留吧. 外层不知道EraseSwap内部细节的情况, 主动设置setLastError能确保有对应的错误信息

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

追加一具评论:Array::UnstableErase()返回是bool也可以,更易使用,但里面其实已经设置了last errro,此处重复设置,需要修正

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

fixed

if (UNLIKELY(!_head))
{
LLBC_SetLastError(LLBC_ERROR_NOT_FOUND);
return LLBC_FAILED;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

有点多余,下面for循环里面判断包括了这个case了

Comment thread llbc/src/core/objbase/Array.cpp Outdated
_objs[lastIdx] = nullptr;
--_size;

LLBC_SetLastError(LLBC_ERROR_SUCCESS);
Copy link
Copy Markdown
Collaborator

@cw7180 cw7180 Jan 10, 2026

Choose a reason for hiding this comment

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

好像有点多余,成功应该不需要SetLastError了

Copy link
Copy Markdown
Owner

@lailongwei lailongwei left a comment

Choose a reason for hiding this comment

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

@cw7180 已经review得很全面,这边额外加了一个review意见,麻烦 @firewiki 再确认下哈
另外这个RemmoveObject调用只发生在:对象异常被强制删除、又在GC池中的情况,可以针对这个再补充一下测试用例

Comment thread llbc/include/llbc/core/objbase/Array.h Outdated
* Erase single object using swap-and-pop strategy (O(1) removal).
* Note: This method does NOT preserve element order.
*/
bool EraseSwap(Obj *o, bool releaseObj);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

建议命名更容易理解:UnstableErase(Obj *o, bool releaseObj)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

ok

Comment thread llbc/src/core/objbase/Array.cpp Outdated
_objs[i]->Release();

// Swap-and-pop: swap with last element and decrement size
// This achieves O(1) removal instead of O(n) memmove
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

最后可加个.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

ok


_arr->Erase(o, false);
// Use EraseSwap for O(1) removal (swap-and-pop strategy)
// Note: EraseSwap does not preserve element order, which is acceptable for AutoReleasePool
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

ok

// Note: EraseSwap does not preserve element order, which is acceptable for AutoReleasePool
if (!_arr->EraseSwap(o, false))
{
LLBC_SetLastError(LLBC_ERROR_NOT_FOUND);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

追加一具评论:Array::UnstableErase()返回是bool也可以,更易使用,但里面其实已经设置了last errro,此处重复设置,需要修正

@firewiki firewiki closed this Jan 29, 2026
@firewiki firewiki reopened this Jan 29, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jan 29, 2026

Image description CodeRabbit


Uplevel your code reviews with CodeRabbit Pro

CodeRabbit Pro

If you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects.

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Image description CodeRabbit

Commits Files that changed from the base of the PR and between d4c5f75 and 330461d commits.
Files selected (5)
  • .github/workflows/linux-build.yml (1)
  • llbc/include/llbc/core/objbase/Array.h (1)
  • llbc/src/core/objbase/Array.cpp (1)
  • llbc/src/core/objbase/AutoReleasePool.cpp (1)
  • llbc/src/core/objbase/AutoReleasePoolStack.cpp (1)
Files not summarized due to errors (5)
  • .github/workflows/linux-build.yml (nothing obtained from openai)
  • llbc/include/llbc/core/objbase/Array.h (nothing obtained from openai)
  • llbc/src/core/objbase/Array.cpp (nothing obtained from openai)
  • llbc/src/core/objbase/AutoReleasePool.cpp (nothing obtained from openai)
  • llbc/src/core/objbase/AutoReleasePoolStack.cpp (nothing obtained from openai)
Files not reviewed due to errors (5)
  • llbc/include/llbc/core/objbase/Array.h (no response)
  • llbc/src/core/objbase/Array.cpp (no response)
  • llbc/src/core/objbase/AutoReleasePoolStack.cpp (no response)
  • .github/workflows/linux-build.yml (no response)
  • llbc/src/core/objbase/AutoReleasePool.cpp (no response)
Review comments generated (0)
  • Review: 0
  • LGTM: 0

Tips

Chat with Image description CodeRabbit Bot (@coderabbitai)

  • Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
  • Invite the bot into a review comment chain by tagging @coderabbitai in a reply.

Code suggestions

  • The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
  • You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.

Pausing incremental reviews

  • Add @coderabbitai: ignore anywhere in the PR description to pause further reviews from the bot.

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.

4 participants