Shawn O. Pearce wrote:
Andreas Ericsson <ae@xxxxxx> wrote:
Pierre Habouzit wrote:
On Fri, Oct 31, 2008 at 06:41:54PM +0000, Shawn O. Pearce wrote:
How about this?
http://www.spearce.org/projects/scm/libgit2/apidocs/CONVENTIONS
FWIW I've read what you say about types, while this is good design to
make things abstract, accessors are slower _and_ disallow many
optimizations as it's a function call and that it may clobber all your
pointers values.
True, accessors slow things down. But I'm not sure that the
accessors at the application level are going to be a huge problem.
Where the CPU time really matters is inside the tight loops of the
library, where we can expose the struct to ourselves, because if
the layout changes we'd be relinking the library anyway with the
updated object code.
I would rather stick with accessors right now. We could in the
future expose the structs and convert the accessors to macros or
inline functions in a future version of the ABI if performance is
really shown to be a problem here from the *application*.
Remember we are mostly talking about applications that are happy to
fork+exec git right now. A little accessor function call is *still*
faster than that fork call was.
struct object in git has not changed since 2006.06. struct commit hasn't
since 2005.04 if you ignore { unsigned int indegree; void *util; } that
if I'm correct are annotations, and is a problem we (I think) have to
address differently anyways (I gave my proposal on this, I'm eager to
hear about what other think on the subject). So if in git.git that _is_
a moving target we have had a 2 year old implementation for those types,
it's that they're pretty well like this.
It's IMNSHO on the matter that core structures of git _will_ have to be
made explicit. I'm thinking objects and their "subtypes" (commits,
trees, blobs). Maybe a couple of things on the same vein.
I agree. "git_commit", "git_tree", "git_blob" and "git_tag" can almost
certainly be set in stone straight away.
Eh, I disagree here. In git.git today "struct commit" exposes its
buffer with the canonical commit encoding. Having that visible
wrecks what Nico and I were thinking about doing with pack v4 and
encoding commits in a non-canonical format when stored in packs.
Ditto with trees.
Err... isn't that backwards? Surely you want to store stuff in the
canonical format so you're forced to do as few translations as
possible? Or are you trying to speed up packing by skipping the
canonicalization part? If so, that would slow down reading (or
rather, presenting) the commits, wouldn't it?
Because git.git code goes against that canonical buffer we cannot
easily insert pack v4 and test the improvements we want to make.
The refactoring required is one of the reasons we haven't done pack
v4 yet. _IF_ we really are going through this effort of building
a different API and shifting to its use in git.git I want to make
sure we at least initially leave the door open to make changes
without rewriting everything *again*.
Well, if macro usage is adhered to one wouldn't have to worry,
since the macro can just be rewritten with a function later (if,
for example, translation or some such happens to be required).
Older code linking to a newer library would work (assuming the
size of the commit object doesn't change anyway), but newer code
linking to an older library would not. Otoh, they wouldn't even
build unless they used the wrong header files, so there is nothing
to worry about there.
Accessor functions can usually be inlined or macro'd away. But
they cannot be magically inserted by the compiler if they aren't
there in the first place. This isn't Python... :-)
What I meant was this (I'm a tad drunk, so read the spirit, not
the letter):
in "foo-api.h":
--%<--%<--
#ifdef BUILDING_FOR_DEPLOYING
#include "git_foo_decls.h"
# define git_foo_get_buf(git_foo) (git_foo->buf)
#else
#include "git_foo_fwd_decls.h"
extern const char *git_foo_get_buf(git_foo *foo);
#endif
--%<--%<--
foo.c
--%<--%<--
#include "foo-api.h"
#include "git_foo_decls.h"
#ifndef BUILDING_FOR_DEPLOYING
const char git_foo_get_buf(git_foo *foo)
{
return foo->buf;
}
/* other accessors go here */
#endif
/* rest of git_foo manipulators go here */
--%<--%<--
It's almost certainly not worth it for libgit2 though, as git@vger
provides a good review system.
--
Andreas Ericsson andreas.ericsson@xxxxxx
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
