You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by GitBox <gi...@apache.org> on 2022/04/29 21:22:59 UTC
[GitHub] [qpid-proton] astitcher opened a new pull request, #369: Object/speed simplify
astitcher opened a new pull request, #369:
URL: https://github.com/apache/qpid-proton/pull/369
This PR incorporates the work in #368 and adds some more simplification of the object system to speed up my benchmarks by 10-15% compared to the main branch commit it is based on.
The speed ups are due to:
* Removing unnecessary reify operations
* Inlining a lot of the object mechanism
* Devirtualizing most of the object function lookups.
* Something else...?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher commented on a diff in pull request #369: Object system speedups and simplifications
Posted by GitBox <gi...@apache.org>.
astitcher commented on code in PR #369:
URL: https://github.com/apache/qpid-proton/pull/369#discussion_r864414337
##########
c/src/core/object/object.c:
##########
@@ -58,50 +67,130 @@ pn_cid_t pn_class_id(const pn_class_t *clazz)
return clazz->cid;
}
+static inline void *pni_object_new(const pn_class_t *clazz, size_t size)
+{
+ void *object = NULL;
+ pni_head_t *head = (pni_head_t *) pni_mem_zallocate(clazz, sizeof(pni_head_t) + size);
+ if (head != NULL) {
+ object = head + 1;
+ head->clazz = clazz;
+ head->refcount = 1;
+ }
+ return object;
+}
+
+static inline void pni_object_incref(void *object) {
+ if (object) {
+ pni_head(object)->refcount++;
+ }
+}
+
+static inline int pni_object_refcount(void *object)
+{
+ assert(object);
+ return pni_head(object)->refcount;
+}
+
+static inline void pni_object_decref(void *object)
+{
+ pni_head_t *head = pni_head(object);
+ assert(head->refcount > 0);
+ head->refcount--;
+}
+
+static inline void pni_object_free(void *object)
+{
+ pni_head_t *head = pni_head(object);
+ pni_mem_deallocate(head->clazz, head);
+}
+
+void pn_object_incref(void *object) {
+ pni_object_incref(object);
+}
+
+static inline void *pni_class_new(const pn_class_t *clazz, size_t size) {
+ return clazz->newinst
+ ? clazz->newinst(clazz, size)
+ : pni_object_new(clazz, size);
+}
+
+static inline void pni_class_incref(const pn_class_t *clazz, void *object) {
+ if (clazz->incref) {
+ clazz->incref(object);
+ } else {
+ pni_object_incref(object);
+ }
+}
+
Review Comment:
I personally find the ternary easier to read! But I'm happy to change it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] codecov-commenter commented on pull request #369: Object system speedups and simplifications
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #369:
URL: https://github.com/apache/qpid-proton/pull/369#issuecomment-1113892537
# [Codecov](https://codecov.io/gh/apache/qpid-proton/pull/369?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#369](https://codecov.io/gh/apache/qpid-proton/pull/369?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a219f6d) into [main](https://codecov.io/gh/apache/qpid-proton/commit/700781bd137bdac9ad0165f96ee2056305c89d67?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (700781b) will **decrease** coverage by `7.85%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## main #369 +/- ##
==========================================
- Coverage 88.36% 80.50% -7.86%
==========================================
Files 47 46 -1
Lines 2397 2344 -53
==========================================
- Hits 2118 1887 -231
- Misses 279 457 +178
```
| [Impacted Files](https://codecov.io/gh/apache/qpid-proton/pull/369?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [ruby/lib/handler/reactor\_messaging\_adapter.rb](https://codecov.io/gh/apache/qpid-proton/pull/369/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvaGFuZGxlci9yZWFjdG9yX21lc3NhZ2luZ19hZGFwdGVyLnJi) | `32.55% <0.00%> (-58.14%)` | :arrow_down: |
| [ruby/lib/handler/messaging\_handler.rb](https://codecov.io/gh/apache/qpid-proton/pull/369/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvaGFuZGxlci9tZXNzYWdpbmdfaGFuZGxlci5yYg==) | `57.14% <0.00%> (-35.72%)` | :arrow_down: |
| [ruby/lib/core/message.rb](https://codecov.io/gh/apache/qpid-proton/pull/369/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvY29yZS9tZXNzYWdlLnJi) | `57.05% <0.00%> (-31.77%)` | :arrow_down: |
| [ruby/lib/util/deprecation.rb](https://codecov.io/gh/apache/qpid-proton/pull/369/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvdXRpbC9kZXByZWNhdGlvbi5yYg==) | `70.58% <0.00%> (-29.42%)` | :arrow_down: |
| [ruby/lib/util/error\_handler.rb](https://codecov.io/gh/apache/qpid-proton/pull/369/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvdXRpbC9lcnJvcl9oYW5kbGVyLnJi) | `59.37% <0.00%> (-28.13%)` | :arrow_down: |
| [ruby/lib/core/uri.rb](https://codecov.io/gh/apache/qpid-proton/pull/369/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvY29yZS91cmkucmI=) | `76.66% <0.00%> (-23.34%)` | :arrow_down: |
| [ruby/lib/codec/data.rb](https://codecov.io/gh/apache/qpid-proton/pull/369/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvY29kZWMvZGF0YS5yYg==) | `76.52% <0.00%> (-19.57%)` | :arrow_down: |
| [ruby/lib/core/condition.rb](https://codecov.io/gh/apache/qpid-proton/pull/369/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvY29yZS9jb25kaXRpb24ucmI=) | `80.64% <0.00%> (-19.36%)` | :arrow_down: |
| [ruby/lib/core/session.rb](https://codecov.io/gh/apache/qpid-proton/pull/369/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvY29yZS9zZXNzaW9uLnJi) | `80.76% <0.00%> (-7.70%)` | :arrow_down: |
| [ruby/lib/core/endpoint.rb](https://codecov.io/gh/apache/qpid-proton/pull/369/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvY29yZS9lbmRwb2ludC5yYg==) | `88.23% <0.00%> (-5.89%)` | :arrow_down: |
| ... and [5 more](https://codecov.io/gh/apache/qpid-proton/pull/369/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-proton/pull/369?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/qpid-proton/pull/369?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [700781b...a219f6d](https://codecov.io/gh/apache/qpid-proton/pull/369?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher commented on a diff in pull request #369: Object system speedups and simplifications
Posted by GitBox <gi...@apache.org>.
astitcher commented on code in PR #369:
URL: https://github.com/apache/qpid-proton/pull/369#discussion_r864414072
##########
c/src/core/object/object.c:
##########
@@ -58,50 +67,130 @@ pn_cid_t pn_class_id(const pn_class_t *clazz)
return clazz->cid;
}
+static inline void *pni_object_new(const pn_class_t *clazz, size_t size)
+{
+ void *object = NULL;
+ pni_head_t *head = (pni_head_t *) pni_mem_zallocate(clazz, sizeof(pni_head_t) + size);
+ if (head != NULL) {
+ object = head + 1;
+ head->clazz = clazz;
+ head->refcount = 1;
+ }
+ return object;
+}
+
+static inline void pni_object_incref(void *object) {
+ if (object) {
Review Comment:
pn_class_incref/decref are actually allowed to be called with a NULL object so they really do need to check for null and do nothing if they get that - the other inlined functions may not need the check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] asfgit merged pull request #369: Object system speedups and simplifications
Posted by GitBox <gi...@apache.org>.
asfgit merged PR #369:
URL: https://github.com/apache/qpid-proton/pull/369
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher commented on a diff in pull request #369: Object system speedups and simplifications
Posted by GitBox <gi...@apache.org>.
astitcher commented on code in PR #369:
URL: https://github.com/apache/qpid-proton/pull/369#discussion_r864414485
##########
c/src/core/object/object.c:
##########
@@ -58,50 +67,130 @@ pn_cid_t pn_class_id(const pn_class_t *clazz)
return clazz->cid;
}
+static inline void *pni_object_new(const pn_class_t *clazz, size_t size)
+{
+ void *object = NULL;
+ pni_head_t *head = (pni_head_t *) pni_mem_zallocate(clazz, sizeof(pni_head_t) + size);
+ if (head != NULL) {
+ object = head + 1;
+ head->clazz = clazz;
+ head->refcount = 1;
+ }
+ return object;
+}
+
+static inline void pni_object_incref(void *object) {
+ if (object) {
+ pni_head(object)->refcount++;
+ }
+}
+
+static inline int pni_object_refcount(void *object)
+{
+ assert(object);
+ return pni_head(object)->refcount;
+}
+
+static inline void pni_object_decref(void *object)
+{
+ pni_head_t *head = pni_head(object);
+ assert(head->refcount > 0);
+ head->refcount--;
+}
+
+static inline void pni_object_free(void *object)
+{
+ pni_head_t *head = pni_head(object);
+ pni_mem_deallocate(head->clazz, head);
+}
+
+void pn_object_incref(void *object) {
+ pni_object_incref(object);
+}
+
+static inline void *pni_class_new(const pn_class_t *clazz, size_t size) {
+ return clazz->newinst
+ ? clazz->newinst(clazz, size)
+ : pni_object_new(clazz, size);
+}
+
+static inline void pni_class_incref(const pn_class_t *clazz, void *object) {
+ if (clazz->incref) {
+ clazz->incref(object);
+ } else {
+ pni_object_incref(object);
+ }
+}
+
+static inline void pni_class_decref(const pn_class_t *clazz, void *object) {
+ if (clazz->decref) {
+ clazz->decref(object);
+ } else {
+ pni_object_decref(object);
+ }
+}
+
+static inline int pni_class_refcount(const pn_class_t *clazz, void *object) {
+ return clazz->refcount
+ ? clazz->refcount(object)
+ : pni_object_refcount(object);
+}
+
+static inline void pni_class_free(const pn_class_t *clazz, void *object) {
+ if (clazz->free) {
+ clazz->free(object);
+ } else {
+ pni_object_free(object);
+ }
+}
+
void *pn_class_new(const pn_class_t *clazz, size_t size)
{
assert(clazz);
- void *object = clazz->newinst(clazz, size);
- if (clazz->initialize) {
+ void *object = pni_class_new(clazz, size);
+ if (object && clazz->initialize) {
clazz->initialize(object);
}
return object;
}
void *pn_class_incref(const pn_class_t *clazz, void *object)
{
- assert(clazz);
if (object) {
- clazz = clazz->reify(object);
- clazz->incref(object);
+ if (clazz==PN_OBJECT) {
Review Comment:
These lines have gone anyway now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] ssorj commented on a diff in pull request #369: Object system speedups and simplifications
Posted by GitBox <gi...@apache.org>.
ssorj commented on code in PR #369:
URL: https://github.com/apache/qpid-proton/pull/369#discussion_r863851716
##########
c/src/core/object/object.c:
##########
@@ -58,50 +67,130 @@ pn_cid_t pn_class_id(const pn_class_t *clazz)
return clazz->cid;
}
+static inline void *pni_object_new(const pn_class_t *clazz, size_t size)
+{
+ void *object = NULL;
+ pni_head_t *head = (pni_head_t *) pni_mem_zallocate(clazz, sizeof(pni_head_t) + size);
+ if (head != NULL) {
+ object = head + 1;
+ head->clazz = clazz;
+ head->refcount = 1;
+ }
+ return object;
+}
+
+static inline void pni_object_incref(void *object) {
+ if (object) {
+ pni_head(object)->refcount++;
+ }
+}
+
+static inline int pni_object_refcount(void *object)
+{
+ assert(object);
+ return pni_head(object)->refcount;
+}
+
+static inline void pni_object_decref(void *object)
+{
+ pni_head_t *head = pni_head(object);
+ assert(head->refcount > 0);
+ head->refcount--;
+}
+
+static inline void pni_object_free(void *object)
+{
+ pni_head_t *head = pni_head(object);
+ pni_mem_deallocate(head->clazz, head);
+}
+
+void pn_object_incref(void *object) {
+ pni_object_incref(object);
+}
+
+static inline void *pni_class_new(const pn_class_t *clazz, size_t size) {
+ return clazz->newinst
+ ? clazz->newinst(clazz, size)
+ : pni_object_new(clazz, size);
+}
+
+static inline void pni_class_incref(const pn_class_t *clazz, void *object) {
+ if (clazz->incref) {
+ clazz->incref(object);
+ } else {
+ pni_object_incref(object);
+ }
+}
+
Review Comment:
Can we have both of these use the if/else form? I find it easier to read.
##########
c/src/core/object/object.c:
##########
@@ -58,50 +67,130 @@ pn_cid_t pn_class_id(const pn_class_t *clazz)
return clazz->cid;
}
+static inline void *pni_object_new(const pn_class_t *clazz, size_t size)
+{
+ void *object = NULL;
+ pni_head_t *head = (pni_head_t *) pni_mem_zallocate(clazz, sizeof(pni_head_t) + size);
+ if (head != NULL) {
+ object = head + 1;
+ head->clazz = clazz;
+ head->refcount = 1;
+ }
+ return object;
+}
+
+static inline void pni_object_incref(void *object) {
+ if (object) {
+ pni_head(object)->refcount++;
+ }
+}
+
+static inline int pni_object_refcount(void *object)
+{
+ assert(object);
+ return pni_head(object)->refcount;
+}
+
+static inline void pni_object_decref(void *object)
+{
+ pni_head_t *head = pni_head(object);
+ assert(head->refcount > 0);
+ head->refcount--;
+}
+
+static inline void pni_object_free(void *object)
+{
+ pni_head_t *head = pni_head(object);
+ pni_mem_deallocate(head->clazz, head);
+}
+
+void pn_object_incref(void *object) {
+ pni_object_incref(object);
+}
+
+static inline void *pni_class_new(const pn_class_t *clazz, size_t size) {
+ return clazz->newinst
+ ? clazz->newinst(clazz, size)
+ : pni_object_new(clazz, size);
+}
+
+static inline void pni_class_incref(const pn_class_t *clazz, void *object) {
+ if (clazz->incref) {
+ clazz->incref(object);
+ } else {
+ pni_object_incref(object);
+ }
+}
+
+static inline void pni_class_decref(const pn_class_t *clazz, void *object) {
+ if (clazz->decref) {
+ clazz->decref(object);
+ } else {
+ pni_object_decref(object);
+ }
+}
+
+static inline int pni_class_refcount(const pn_class_t *clazz, void *object) {
+ return clazz->refcount
+ ? clazz->refcount(object)
+ : pni_object_refcount(object);
+}
+
+static inline void pni_class_free(const pn_class_t *clazz, void *object) {
Review Comment:
A bigger question about naming conventions. These are static in a library file. Should they use pni_? In some other places, they have no prefix. (I've long been confused about this.)
##########
c/src/core/object/object.c:
##########
@@ -58,50 +67,130 @@ pn_cid_t pn_class_id(const pn_class_t *clazz)
return clazz->cid;
}
+static inline void *pni_object_new(const pn_class_t *clazz, size_t size)
+{
+ void *object = NULL;
+ pni_head_t *head = (pni_head_t *) pni_mem_zallocate(clazz, sizeof(pni_head_t) + size);
+ if (head != NULL) {
+ object = head + 1;
+ head->clazz = clazz;
+ head->refcount = 1;
+ }
+ return object;
+}
+
+static inline void pni_object_incref(void *object) {
+ if (object) {
+ pni_head(object)->refcount++;
+ }
+}
+
+static inline int pni_object_refcount(void *object)
+{
+ assert(object);
+ return pni_head(object)->refcount;
+}
+
+static inline void pni_object_decref(void *object)
+{
+ pni_head_t *head = pni_head(object);
+ assert(head->refcount > 0);
+ head->refcount--;
+}
+
+static inline void pni_object_free(void *object)
+{
+ pni_head_t *head = pni_head(object);
+ pni_mem_deallocate(head->clazz, head);
+}
+
+void pn_object_incref(void *object) {
+ pni_object_incref(object);
+}
+
+static inline void *pni_class_new(const pn_class_t *clazz, size_t size) {
+ return clazz->newinst
+ ? clazz->newinst(clazz, size)
+ : pni_object_new(clazz, size);
+}
+
+static inline void pni_class_incref(const pn_class_t *clazz, void *object) {
+ if (clazz->incref) {
+ clazz->incref(object);
+ } else {
+ pni_object_incref(object);
+ }
+}
+
+static inline void pni_class_decref(const pn_class_t *clazz, void *object) {
+ if (clazz->decref) {
+ clazz->decref(object);
+ } else {
+ pni_object_decref(object);
+ }
+}
+
+static inline int pni_class_refcount(const pn_class_t *clazz, void *object) {
+ return clazz->refcount
+ ? clazz->refcount(object)
+ : pni_object_refcount(object);
+}
+
+static inline void pni_class_free(const pn_class_t *clazz, void *object) {
+ if (clazz->free) {
+ clazz->free(object);
+ } else {
+ pni_object_free(object);
+ }
+}
+
void *pn_class_new(const pn_class_t *clazz, size_t size)
{
assert(clazz);
- void *object = clazz->newinst(clazz, size);
- if (clazz->initialize) {
+ void *object = pni_class_new(clazz, size);
+ if (object && clazz->initialize) {
clazz->initialize(object);
}
return object;
}
void *pn_class_incref(const pn_class_t *clazz, void *object)
{
- assert(clazz);
if (object) {
- clazz = clazz->reify(object);
- clazz->incref(object);
+ if (clazz==PN_OBJECT) {
Review Comment:
Here and elsewhere, I would prefer space around ==.
##########
c/src/core/object/object.c:
##########
@@ -58,50 +67,130 @@ pn_cid_t pn_class_id(const pn_class_t *clazz)
return clazz->cid;
}
+static inline void *pni_object_new(const pn_class_t *clazz, size_t size)
+{
+ void *object = NULL;
+ pni_head_t *head = (pni_head_t *) pni_mem_zallocate(clazz, sizeof(pni_head_t) + size);
+ if (head != NULL) {
+ object = head + 1;
+ head->clazz = clazz;
+ head->refcount = 1;
+ }
+ return object;
+}
+
+static inline void pni_object_incref(void *object) {
+ if (object) {
+ pni_head(object)->refcount++;
+ }
+}
+
+static inline int pni_object_refcount(void *object)
+{
+ assert(object);
+ return pni_head(object)->refcount;
+}
+
+static inline void pni_object_decref(void *object)
+{
+ pni_head_t *head = pni_head(object);
+ assert(head->refcount > 0);
+ head->refcount--;
+}
+
+static inline void pni_object_free(void *object)
+{
+ pni_head_t *head = pni_head(object);
+ pni_mem_deallocate(head->clazz, head);
+}
+
+void pn_object_incref(void *object) {
+ pni_object_incref(object);
+}
+
+static inline void *pni_class_new(const pn_class_t *clazz, size_t size) {
+ return clazz->newinst
+ ? clazz->newinst(clazz, size)
+ : pni_object_new(clazz, size);
+}
+
+static inline void pni_class_incref(const pn_class_t *clazz, void *object) {
+ if (clazz->incref) {
+ clazz->incref(object);
+ } else {
+ pni_object_incref(object);
+ }
+}
+
Review Comment:
Same for pni_class_refcount below.
##########
c/src/core/object/object.c:
##########
@@ -58,50 +67,130 @@ pn_cid_t pn_class_id(const pn_class_t *clazz)
return clazz->cid;
}
+static inline void *pni_object_new(const pn_class_t *clazz, size_t size)
+{
+ void *object = NULL;
+ pni_head_t *head = (pni_head_t *) pni_mem_zallocate(clazz, sizeof(pni_head_t) + size);
+ if (head != NULL) {
+ object = head + 1;
+ head->clazz = clazz;
+ head->refcount = 1;
+ }
+ return object;
+}
+
+static inline void pni_object_incref(void *object) {
+ if (object) {
Review Comment:
Should this assert instead of null check? _refcount and _decref are (_decref needs an explicit assert, I think). Maybe the null check in pn_object_incref, not pni_object_incref?
In general, I'd prefer all the refcounting methods required a non-null object, but that does require fixing up a number of places.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] ssorj commented on a diff in pull request #369: Object system speedups and simplifications
Posted by GitBox <gi...@apache.org>.
ssorj commented on code in PR #369:
URL: https://github.com/apache/qpid-proton/pull/369#discussion_r864735197
##########
c/src/proactor/win_iocp.cpp:
##########
@@ -1514,7 +1514,7 @@ void pni_iocp_initialize(void *obj)
pni_shared_pool_create(iocp);
iocp->completion_port = CreateIoCompletionPort(INVALID_HANDLE_VALUE, NULL, 0, 0);
assert(iocp->completion_port != NULL);
- iocp->zombie_list = pn_list(PN_OBJECT, 0);
+ iocp->zombie_list = pn_list(PN_CLASSCLASS(pni_iocpdesc), 0);
Review Comment:
I noticed that this one is not like the others, so checking if it's as it should be. The others pass the address of PN_CLASSCLASS.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher commented on pull request #369: Object system speedups and simplifications
Posted by GitBox <gi...@apache.org>.
astitcher commented on PR #369:
URL: https://github.com/apache/qpid-proton/pull/369#issuecomment-1113752585
I'm pretty happy with the state of this PR. I think the commits need to be somewhat changed for coherence and comprehension. But it is essentially ready to merge
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] ssorj commented on a diff in pull request #369: Object system speedups and simplifications
Posted by GitBox <gi...@apache.org>.
ssorj commented on code in PR #369:
URL: https://github.com/apache/qpid-proton/pull/369#discussion_r863849737
##########
c/src/core/object/object.c:
##########
@@ -58,50 +67,130 @@ pn_cid_t pn_class_id(const pn_class_t *clazz)
return clazz->cid;
}
+static inline void *pni_object_new(const pn_class_t *clazz, size_t size)
+{
+ void *object = NULL;
+ pni_head_t *head = (pni_head_t *) pni_mem_zallocate(clazz, sizeof(pni_head_t) + size);
+ if (head != NULL) {
+ object = head + 1;
+ head->clazz = clazz;
+ head->refcount = 1;
+ }
+ return object;
+}
+
+static inline void pni_object_incref(void *object) {
+ if (object) {
Review Comment:
Should this assert instead of null check? _refcount and _decref do (_decref needs an explicit assert, I think). Maybe the null check in pn_object_incref, not pni_object_incref?
In general, I'd prefer all the refcounting methods required a non-null object, but that does require fixing up a number of places.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher commented on a diff in pull request #369: Object system speedups and simplifications
Posted by GitBox <gi...@apache.org>.
astitcher commented on code in PR #369:
URL: https://github.com/apache/qpid-proton/pull/369#discussion_r864762710
##########
c/src/proactor/win_iocp.cpp:
##########
@@ -1514,7 +1514,7 @@ void pni_iocp_initialize(void *obj)
pni_shared_pool_create(iocp);
iocp->completion_port = CreateIoCompletionPort(INVALID_HANDLE_VALUE, NULL, 0, 0);
assert(iocp->completion_port != NULL);
- iocp->zombie_list = pn_list(PN_OBJECT, 0);
+ iocp->zombie_list = pn_list(PN_CLASSCLASS(pni_iocpdesc), 0);
Review Comment:
Good spot - that's probably my windows build failure right there - thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org