Closed Bug 458393 Opened 16 years ago Closed 16 years ago

TC PortAPI strategy needs formalization

Categories

(Tamarin Graveyard :: Virtual Machine, defect)

Product:
Tamarin Graveyard
Component:
Virtual Machine
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
VERIFIED DUPLICATE of bug 467596

People

(Reporter: stejohns, Unassigned)

Details

portapi_avmbuild.h and various things provided by it has recently gotten introduced as a way to offer platform customization in a less invasive way. this bug is a tracking point to discuss the naming conventions, usage, etc that are occurring, to ensure we have consensus on it before usage gets too pervasive.
Getting the ball rolling:

(1) "AVMPlus_PortAPI_XXX" a naming convention we really want to propagate? Seems awkward and long to me. Unique prefixes are good (especially for macros) but this seems like overkill. 

(2) If portapi.h is a good way to go, should we include it unconditionally and move all appropriate items into it?
Please read the doc on the wiki:

https://wiki.mozilla.org/Tamarin:Porting
Ah, nice, I hadn't realized this was written up in the wiki -- my bad.

Let's leave this open for a few days to see if anyone else has comments or concerns -- if not, I'll close this out.
I do agree that the naming convention is awkward, but we can't use namespaces in macros (or can we?) and I wanted to be sure to avoid collisions with macros on other platforms. I wanted to use PAPI instead of PortAPI, but PAPI has already been claimed for several other uses. We could skip the module part and just use PortAPI_XXX, but I thought the module prefix helps make it clear where the function/macro/etc is defined and used. If anyone has other suggestions, I'd be happy to update the wiki and the code.
The design is 100% right but the naming problem is real IMO: names are too long and visually very busy due to mixed case.  I think the right balance is a single 2-6 letter upper case prefix that is easily recognized and visually filtered.  I'm not sure the "avmplus" module name is relevant, because presumably if we provide a method for flushing the instruction cache it doesn't matter who it's provided to, only what it's called.  And if the prefix is not mistaken for something else then the "PortAPI" infix is not necessary either.

How about ADBE_ as the prefix?  ADBE_FlushInstructionCache, ADBE_memcpy (when that becomes necessary), and so on.  "AVMPLUS_" and "AVMPORT_" are too long IMO, and the former is already used for feature configuration.  "AVMP" would do in a pinch as it's unlikely to clash, but it has little mnemonic value.  "AVMPI" (AVM Porting Interface) might be slightly better but again, for all of these I don't know that it's important that AVM be a part of the name, quite the contrary.
how about VMPI - it's a virtual machine porting interface.  (or PPI for platform PI, or MPI, for machine PI)  i'd shy away from ADBE, experience is that the lifetime of the ticker symbol is shorter than the lifetime of the code.  I do like AVMPI better than ADBE

+1 on using the "normal" api name, too, it is very clear when it maps well.  VMPI_alloca(), etc.
-1 on ADBE as the prefix, both for practical and philosophical reasons.

Normally I prefer prefixes of this sort to be only 2 or 3 chars, but 4 is probably pragmatic here to avoid collisions -- VMPI sounds reasonable to me. A quick google for it shows it currently used for "Valve Message Passing Interface" in the Source engine, which probably isn't likely to overlap our code anytime soon.
I can live with "VMPI".  In favor of the longer "AVMPI" is that "AVM" (actually "AVMPLUS") is used as the prefix on all defines configuring the VM, and the ongoing coding standards work suggests propagating this meme.  But again, "VMPI" is OK by me.
We could always go with "AVMP_" (AVM Porting) as the prefix.

My objection to "AVMPI_" is solely the length; IMHO a 4-char prefix is already pushing it, readability wise. But I could probably be persuaded for "AVMPI_".
I agree five is overlong.  If forced to choose, probably VMPI_ over AVMP_ because the former rolls off the tongue more easily; thus, it may therefore be easier to filter it when reading code.  (Works for me anyway :)
is the idea of PORTABLE_ d.o.a?  sure is easy to read, even if long.
I think the length is a problem, esp with the boldface, your eye gravitates too much toward it.

Silly example:

  if (PORTABLE_itoa(icache) > 0)
      PORTABLE_FlushInstructionCache(from,to)

vs

  if (VMPI_itoa(icache) > 0)
      VMPI_FlushInstructionCache(from,to)

I think Steven is right, four letters is pushing it.  Two would be ideal, but may need either some elaborate ifdeffery / header file setup or coordination with the embedder to avoid conflicts.

  if (PI_itoa(icache) > 0)
      PI_FlushInstructionCache(from,to)
(In reply to comment #6)
> +1 on using the "normal" api name, too, it is very clear when it maps well. 
> VMPI_alloca(), etc.

It doesn't map as well when you need to also have a reciprocal call to free.
Would the combination of VMPI_alloca() VMPI_alloca_free() be more meaningful
that VMPI_StackAlloc() and VMI_StackFree()?

And yes, we could make the non-alloca version of VMPI_alloca use a wrapper that
deallocates the memory when the function returns, but I'd rather keep the
macros as simple as possible rather than try to exactly replicate the behavior of a function that we're trying to replace.
(In reply to comment #13)
> It doesn't map as well when you need to also have a reciprocal call to free.
> Would the combination of VMPI_alloca() VMPI_alloca_free() be more meaningful
> that VMPI_StackAlloc() and VMI_StackFree()?

I see your point, and I don't feel super strongly but here, but the more i think about it, VMPI_StackAlloc having to have a matching VMPI_StackFree is kind of a contradiction in terms, we should be using VMPI_malloc and VMPI_free, or VMPI_alloca by itself (however the auto-free happens).

anyway, i didn't mean to pick on alloca in making the more general point of doing our best to keep names short + simple, mirrorring libc names & semantics where possible.
Any reason we can't use lowercase?

avmp_alloc() 
avmp_free()

looks better to me.

btw, google code search shows hits for both vmpi_ and vmi_
(In reply to comment #13)
> (In reply to comment #6)
> > +1 on using the "normal" api name, too, it is very clear when it maps well. 
> > VMPI_alloca(), etc.
> 
> It doesn't map as well when you need to also have a reciprocal call to free.
> Would the combination of VMPI_alloca() VMPI_alloca_free() be more meaningful
> that VMPI_StackAlloc() and VMI_StackFree()?
> 
> And yes, we could make the non-alloca version of VMPI_alloca use a wrapper that
> deallocates the memory when the function returns, but I'd rather keep the
> macros as simple as possible rather than try to exactly replicate the behavior
> of a function that we're trying to replace.

I think alloca was a particularly poorly chosen example of Edwin's :-)  I say that because there is no way we can emulate it transparently, as it ties in to the compiler / run-time system.  For most library functions, the principle holds, however.

As I wrote in other mail, I think alloca should simply be banned from the VM source code, and I think that we should not try to emulate it.  If we want to provide some heap allocation mechanism with automatic freeing on return or exception then that's something else, but C++ has auto_ptr for that and that would be the mechanism to mimic, not alloca.
(In reply to comment #15)
> Any reason we can't use lowercase?
> 
> avmp_alloc() 
> avmp_free()
> 
> looks better to me.
> 
> btw, google code search shows hits for both vmpi_ and vmi_

I'm not dogmatic about this, and grepping for "vmpi_" rather than "VMPI_" in source code is unlikely to have more false positives, but I prefer upper case because it stands out and therefore can be filtered visually more easily.  With lower case the expression looks like a function call; with upper case it's clear that something interesting is going on, namely, these are always macro calls and may map to functions of other names.   (BTW we should agree on whether the invocations always have function semantics or are subject to multiple evaluation problems a la "max" and "min".  I assume the former.)
+1 to CAPS_ rather than caps_; I also am not dogmatic but find it more visually useful.

IMHO function semantics are a hard requirement -- anyone who doesn't know about the do{}while(0) idiom needs to be made to learn it.
Agree on function semantics.  Note that in that case most of the porting interface entry points can be used in a expression contexts, so the do..while(0) trick will not help you.  In most cases the macro will expand to a trivial result (the success result), a (void)0 for void APIs, or to a proper call.

So the proposal on the table is for a porting API where each member:

  - is defined by a macro
  - has function semantics (arguments evaluated once, may appear in expressions except when the result is void)
  - has a name prefixed by "VMPI_"

Two issues with "function semantics":

* Note that the "defined by a macro" means that the address of an API function may never be taken because the expansion may not be a function.)

* Do we require arguments to be evaluated exactly once (true function semantics) or at most once (if the macro expands to something trivial)?
> * Note that the "defined by a macro" means that the address of an API function
> may never be taken because the expansion may not be a function.)

Not a problem IMHO.
 
> * Do we require arguments to be evaluated exactly once (true function
> semantics) or at most once (if the macro expands to something trivial)?

Hmm... I'm tempted to say at-most-once. Careless use could lead to odd side-effects though.
(In reply to comment #20)
> 
> > * Do we require arguments to be evaluated exactly once (true function
> > semantics) or at most once (if the macro expands to something trivial)?
> 
> Hmm... I'm tempted to say at-most-once. Careless use could lead to odd
> side-effects though.

Indeed.  And you can make it arbitrarily hairy if some expansions use some but not all of the arguments.

I think the viable definitions are (a) all arguments are always evaluated and (b) each argument, considered individually, may be evaluated zero or more times.  The latter is more dangerous but also slightly more convenient since computations that only need to be performed if the call is not a no-op can be placed inside the call and will not require a separate ifdef.  And, most programmers are probably familiar with the problem from the use of assert() anyway, so we're not introducing any new intellectual machinery in a sense.

My vote, like yours, is for (b).
VMPI_ macros it is, at-most-once evaluation semantics.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → DUPLICATE
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.