On 11/28/2010 01:44 PM, Michael S. Tsirkin wrote:
On Sun, Nov 28, 2010 at 11:54:26AM +0200, Avi Kivity wrote:
> On 11/28/2010 11:50 AM, Michael S. Tsirkin wrote:
> >> >
> >> >Another problem is that there seem to be two memory allocations and a
> >> >copy here, apparently just to simplify error handling. It might be fine
> >> >for this test but won't scale for when performance matters.
> >>
> >> When it matters, we can fix it. I don't see msr read/write becoming
> >> a hot path.
> >
> >It will be very painful to fix it.
>
> Why?
Because the API returns a vector.
Returning an object does not involve a copy (return value optimization).
> >>
> >> The compiler should optimize it away completely.
> >
> >Should as opposed to does. Want me to try a simple test?
>
> Please.
Just for fun: optimize for size, and compare code sizes.
The C++ code is yours but I have removed all use of STL to make
it more of an even fight. I checked by object and executable size.
Note that this is shared library build: a C++ executable
will pull in a large C++ library, a C executable won't.
If you are interested in an STL based example let me know.
You can take it from here and make it more real if you like,
or look at the assembler output.
------------------------------
[mst@tuck ~]$ cat test.c
#include<sys/ioctl.h>
#include<sys/types.h>
#include<sys/stat.h>
#include<fcntl.h>
#include<unistd.h>
#include<errno.h>
int main(int argc, char **argv)
{
int fd = open("/dev/kvm", O_RDWR);
int r;
if (fd< 0)
goto open_err;
r = ioctl(fd, 0, 0);
if (r< 0)
goto ioctl_err;
return 0;
ioctl_err:
close(fd);
open_err:
return -1;
}
This code is not reusable. Everywhere you use an fd, you have to repeat
this code.
[mst@tuck ~]$ gcc -c -Os test.c
[mst@tuck ~]$ size test.o
text data bss dec hex filename
97 0 0 97 61 test.o
[mst@tuck ~]$ gcc -Os test.c
[mst@tuck ~]$ size a.out
text data bss dec hex filename
1192 260 8 1460 5b4 a.out
[mst@tuck ~]$ wc -l test.c
22 test.c
------------------------------
[mst@tuck ~]$ cat kvmxx.cpp
extern "C" {
#include<sys/ioctl.h>
#include<sys/types.h>
#include<sys/stat.h>
#include<fcntl.h>
#include<unistd.h>
#include<errno.h>
}
namespace kvm {
class fd {
public:
explicit fd(const char *path, int flags);
~fd() { ::close(_fd); }
long ioctl(unsigned nr, long arg);
private:
int _fd;
};
};
namespace kvm {
static long check_error(long r)
{
if (r == -1) {
throw errno;
}
return r;
}
fd::fd(const char *device_node, int flags)
: _fd(::open(device_node, flags))
{
check_error(_fd);
}
long fd::ioctl(unsigned nr, long arg)
{
return check_error(::ioctl(_fd, nr, arg));
}
}
int main(int ac, char **av)
{
try {
kvm::fd fd("/dev/kvm", O_RDWR);
fd.ioctl(0, 0);
} catch (...) {
return -1;
}
return 0;
}
class kvm::fd is reusable, if you embed it in another object you don't
have to worry about errors any more (as long as the object's methods are
exception safe).
[mst@tuck ~]$ g++ -c -Os kvmxx.cpp
[mst@tuck ~]$ size kvmxx.o
text data bss dec hex filename
529 0 0 529 211 kvmxx.o
[mst@tuck ~]$ g++ -Os kvmxx.cpp
[mst@tuck ~]$ size a.out
text data bss dec hex filename
2254 308 16 2578 a12 a.out
[mst@tuck ~]$ wc kvmxx.cpp
56 kvmxx.cpp
------------------------------
One interesting thing is that the object size grew
faster than linked executable size.
This might mean that the compiler generated some
dead code that the linker then threw out.
It's also interesting that C++ managed to use up
more data/bss storage.
C++ will have much larger data and code sizes because it uses DWARF
tables for unwinding and generates stack unwinding code. These are all
out of the hot path.
> >> There's been a lot
> >> of work in gcc on that.
> >>
> >> About compile times, I don't care much.
> >
> >I do. You will too when we have codebase that can be built as fast as
> >we commit things, so buildbot breaks.
> >This is common in C++ based projects.
>
> If kvm-unit-tests.git takes to long to compile, I'll be very happy.
If the claim is "it's so small it does not matter what it's written in"
then I guess don't mind. But then - why do we care about
error handling so much? For the test, it's probably ok to just assert
after each ioctl/malloc and be done with it.
Yes, all the correctness is more or less pointless here. Like I said,
this is an experiment to see what things look like. I guess each side
will it as proving its claims.
--
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
