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 ets:update_counter #1406

Merged

Conversation

TheSobkiewicz
Copy link
Contributor

@TheSobkiewicz TheSobkiewicz commented Dec 17, 2024

Currently this solution doesn't support list of UpdateOp to update multiple counters.
Based on:
#1405

https://www.erlang.org/docs/23/man/ets#update_counter-3

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

@TheSobkiewicz TheSobkiewicz changed the title Thesobkiewicz/nifs/ets/update counter Add ets:update_counter Dec 17, 2024
@TheSobkiewicz TheSobkiewicz force-pushed the thesobkiewicz/nifs/ets/update_counter branch from 0be618b to 4d64c65 Compare December 19, 2024 13:09
@TheSobkiewicz TheSobkiewicz force-pushed the thesobkiewicz/nifs/ets/update_counter branch 6 times, most recently from 8b5e005 to ce7fa79 Compare January 9, 2025 15:45
@TheSobkiewicz TheSobkiewicz force-pushed the thesobkiewicz/nifs/ets/update_counter branch 3 times, most recently from ecfc907 to 4522c6b Compare February 14, 2025 15:02
case EtsBadEntry:
RAISE_ERROR(BADARG_ATOM);
case EtsAllocationFailure:
RAISE_ERROR(MEMORY_ATOM);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you mean OUT_OF_MEMORY_ATOM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I also refactor other ets functions? Most of them are raising MEMORY_ATOM in this place

Copy link
Collaborator

Choose a reason for hiding this comment

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

let's do that on a different PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I will also format ets_table line to be shorter there ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TheSobkiewicz TheSobkiewicz force-pushed the thesobkiewicz/nifs/ets/update_counter branch from 4522c6b to c662a19 Compare February 14, 2025 16:59
@TheSobkiewicz TheSobkiewicz force-pushed the thesobkiewicz/nifs/ets/update_counter branch 2 times, most recently from f807e22 to 9ca0d2c Compare February 17, 2025 13:51
Copy link
Collaborator

@bettio bettio left a comment

Choose a reason for hiding this comment

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

Let's also check all calls term_to_int(), term_to_int() -> avm_int_t

@TheSobkiewicz TheSobkiewicz force-pushed the thesobkiewicz/nifs/ets/update_counter branch from 9ca0d2c to 056e8be Compare February 17, 2025 14:54
Signed-off-by: Tomasz Sobkiewicz <tomasz.sobkiewicz@swmansion.com>
@TheSobkiewicz TheSobkiewicz force-pushed the thesobkiewicz/nifs/ets/update_counter branch from 056e8be to 2e927e7 Compare February 17, 2025 14:55
@bettio bettio merged commit 19e9d1a into atomvm:main Feb 17, 2025
105 of 110 checks passed
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.

2 participants