[
https://issues.apache.org/jira/browse/DISPATCH-2178?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17366631#comment-17366631
]
ASF GitHub Bot commented on DISPATCH-2178:
------------------------------------------
jiridanek commented on a change in pull request #1267:
URL: https://github.com/apache/qpid-dispatch/pull/1267#discussion_r655430987
##########
File path: src/server.c
##########
@@ -1738,7 +1738,7 @@ void qd_connector_decref(qd_connector_t* connector)
}
}
-__attribute__((noinline)) // permit replacement by dummy implementation in
unit_tests
+__attribute__((weak)) // permit replacement by dummy implementation in
unit_tests
Review comment:
I'd love to do something about it, but there isn't really an existing
universal solution (in the form of a library) that I'd be happy with. And I
never spent the time.
Doing linking substitution like this actually is not so bad. It is even
recommended (in it's original version, with a shared library) in
http://blog.wingman-sw.com/linker-substitution-in-c-limitations-and-workarounds.
When switching from shared library to a static one, I could've either kept
building the so in addition, just for use in tests (that is a good idea, AFAIK,
for the future), or change the tests to use the other technique mentioned in
the article (the pointer substitution one). Or use weak symbols, which produces
the smallest diff.
I'm actually happy that we can get rid of `noinline`, that was bugging me.
Clang by default wants to inline one of the functions on -O2 and we have to
prevent that because of our tests. I feel better about having weak symbols
around, but that might be just me. Maybe there is some harm in them that I am
just not seeing. (Besides the obvious: you might redefine it when you don't
mean to.) Noinline is btw necessary with the runtime injection APIs. Or compile
for tests extra and turn off optimizations altogether.
Regarding the function pointer substitution technique from the blog. It is
the first time I saw it and it is actually a very good solution, also it uses
standard C without any platform magic (either linker trickery or ELF file
manipulation). It looks really simple to understand, but it's actually quite
clever. I wouldn't 've come up with it myself.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
> Do not build separate libqpid-dispatch.so
> -----------------------------------------
>
> Key: DISPATCH-2178
> URL: https://issues.apache.org/jira/browse/DISPATCH-2178
> Project: Qpid Dispatch
> Issue Type: Improvement
> Affects Versions: 1.16.0
> Reporter: Jiri Daněk
> Assignee: Jiri Daněk
> Priority: Major
>
> h3. Motivation
> The split into a binary and a library is not unprecedented (e.g. CPython does
> exactly the same thing), but in Dispatch it is avoidable with minimal code
> changes (an executable can {{dlopen(NULL, ...)}} itself.
> Getting rid of libqpid-dispatch.so will mean one less file that we need to
> install (and since it is not a published API, finding a good place for the so
> has been tricky in the past (see DISPATCH-194).
> Furthermore, avoiding the indirection of a library call will likely give some
> performance benefit at a low level (individual function calls) similar to
> (again) Python in https://fedoraproject.org/wiki/Changes/PythonStaticSpeedup.
> The gains can be best utilized after doing LTO (see DISPATCH-2121).
> h3. Expected issues
> The problem with this is that we lose semantic interposition
> (https://fedoraproject.org/wiki/Changes/PythonNoSemanticInterpositionSpeedup),
> meaning we won't be able to simply override symbols from libqpid-dispatch.so
> at runtime. This is an issue because Dispatch unittests depend on this. (See
> DISPATCH-1783)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
