Thread: Platforms with v6.3 trouble
From what I understand, there are at least two platforms which are having trouble with the macro inlining in v6.3. The alpha ports have trouble inlining the slock assembler code, and the SCO port has trouble inlining the cache lookup code. Since these macros were inlined only for performance reasons, would it be possible to revert to non-inline function calls for these platforms? It would seem that substituting a macro expansion for a compiled routine could be done with a compiler switch (e.g. USE_INLINING) so it could be turned on and off at will. For most of us, the performance gains are fantastic, but for those ports which broke performance has degraded to zero :( - Tom
> > >From what I understand, there are at least two platforms which are > having trouble with the macro inlining in v6.3. The alpha ports have > trouble inlining the slock assembler code, and the SCO port has trouble > inlining the cache lookup code. > > Since these macros were inlined only for performance reasons, would it > be possible to revert to non-inline function calls for these platforms? > It would seem that substituting a macro expansion for a compiled routine > could be done with a compiler switch (e.g. USE_INLINING) so it could be > turned on and off at will. > > For most of us, the performance gains are fantastic, but for those ports > which broke performance has degraded to zero :( Yes, how do we do that? Do we have inlined-versions of these files? Sounds messy. Can people run cpp separately on the files, then compile them? I wonder. I think this is an SCO-only problem, and seeing as their native compilers are notoriously buggy (Microsoft/SVr4 code), it is no wonder. The alpha problem has been solved by having a s_lock.c file, that only contains the alpha/linux locking code. They don't have local asm labels, and hence the workaround. I believe this is not a problem issue for 6.3. Anyone? Of course, we still have the initdb problem, or do we? -- Bruce Momjian | 830 Blythe Avenue maillist@candle.pha.pa.us | Drexel Hill, Pennsylvania 19026 + If your life is a hard drive, | (610) 353-9879(w) + Christ can be your backup. | (610) 853-3000(h)
> > Since these macros were inlined only for performance reasons, would it > > be possible to revert to non-inline function calls for these platforms? > > It would seem that substituting a macro expansion for a compiled routine > > could be done with a compiler switch (e.g. USE_INLINING) so it could be > > turned on and off at will. > > > > For most of us, the performance gains are fantastic, but for those ports > > which broke performance has degraded to zero :( > > Yes, how do we do that? Do we have inlined-versions of these files? > Sounds messy. Can people run cpp separately on the files, then compile > them? I wonder. I think this is an SCO-only problem, and seeing as > their native compilers are notoriously buggy (Microsoft/SVr4 code), it > is no wonder. Well, those macros used to be a function call, right? So surround the macro with#ifdef USE_INLINING #define ... #endif and surround the old subroutine code with #ifndef USE_INLINING ... #endif Or are the macros of a different nature and not just a subroutine inlining? If there still needs to be a little macro expansion, then that could be done also... > The alpha problem has been solved by having a s_lock.c file, that only > contains the alpha/linux locking code. They don't have local asm > labels, and hence the workaround. I believe this is not a problem issue > for 6.3. Anyone? Of course, we still have the initdb problem, or do > we? Don't know...
> Well, those macros used to be a function call, right? So surround the macro > with#ifdef USE_INLINING > #define ... > #endif > > and surround the old subroutine code with > > #ifndef USE_INLINING > ... > #endif > I was trying to avoid having the code in two places, and you can't just copy the macro. You have to replace the parameters in each instance for the huge conditional to work. -- Bruce Momjian | 830 Blythe Avenue maillist@candle.pha.pa.us | Drexel Hill, Pennsylvania 19026 + If your life is a hard drive, | (610) 353-9879(w) + Christ can be your backup. | (610) 853-3000(h)
On the SCO UNIXWARE (UNIVEL) port, it is only necessary to replace the macro definition of fastgetattr with a static function in heapam.h in order to get the code to compile. I guess the people who wrote the compile could not concieve of anyone nesting the trinary operator (?:) to such a depth :-). The UNIXWARE compiler does an excellent job of in-lining the function on it's own without the macro. The patch for the version of heapam.h I am using follows (I am currently using USE_UNIVEL_CC_ASM as the trigger, but that can be changed). Bruce, will this change work? I am not as familiar with this section of code as I would like to be. ---- *** src/include/access/heapam.h.orig Fri Feb 13 20:18:33 1998 --- src/include/access/heapam.h Sat Feb 14 09:03:40 1998 *************** *** 88,93 **** --- 88,142 ---- * * ---------------- */ + #if defined(USE_UNIVEL_CC_ASM) + extern Datum nocachegetattr(HeapTuple tup, int attnum, + TupleDesc att, bool *isnull); + + static Datum fastgetattr(HeapTuple tup, int attnum, TupleDesc tupleDesc, + bool *isnull) + { + return ( + (attnum) > 0 ? + ( + ((isnull) ? (*(isnull) = false) : (dummyret)NULL), + HeapTupleNoNulls(tup) ? + ( + ((tupleDesc)->attrs[(attnum)-1]->attcacheoff != -1 || + (attnum) == 1) ? + ( + (Datum)fetchatt(&((tupleDesc)->attrs[(attnum)-1]), + (char *) (tup) + (tup)->t_hoff + + ( + ((attnum) != 1) ? + (tupleDesc)->attrs[(attnum)-1]->attcacheoff + : + 0 + ) + ) + ) + : + nocachegetattr((tup), (attnum), (tupleDesc), (isnull)) + ) + : + ( + att_isnull((attnum)-1, (tup)->t_bits) ? + ( + ((isnull) ? (*(isnull) = true) : (dummyret)NULL), + (Datum)NULL + ) + : + ( + nocachegetattr((tup), (attnum), (tupleDesc), (isnull)) + ) + ) + ) + : + ( + (Datum)NULL + ) + ); + } + #else #define fastgetattr(tup, attnum, tupleDesc, isnull) \ ( \ AssertMacro((attnum) > 0) ? \ *************** *** 129,136 **** (Datum)NULL \ ) \ ) ! ! /* ---------------- * heap_getattr --- 178,184 ---- (Datum)NULL \ ) \ ) ! #endif /* ---------------- * heap_getattr -- ____ | Billy G. Allie | Domain....: Bill.Allie@mug.org | /| | 7436 Hartwell | Compuserve: 76337,2061 |-/-|----- | Dearborn, MI 48126| MSN.......: B_G_Allie@email.msn.com |/ |LLIE | (313) 582-1540 |
> > On the SCO UNIXWARE (UNIVEL) port, it is only necessary to replace the macro > definition of fastgetattr with a static function in heapam.h in order to get > the code to compile. I guess the people who wrote the compile could not > concieve of anyone nesting the trinary operator (?:) to such a depth :-). The > UNIXWARE compiler does an excellent job of in-lining the function on it's own > without the macro. The patch for the version of heapam.h I am using follows > (I am currently using USE_UNIVEL_CC_ASM as the trigger, but that can be > changed). > > Bruce, will this change work? I am not as familiar with this section of code > as I would like to be. This is fine, and a good place to put it, though the port-specific change should go AFTER the standard #define, not before it, so you do: #if !defined(SCO) #define #else static ... #endif As far as them never suspecting such a macro, well, I never suspected I would ever write such a macro either. But I did, and it works. I didn't inline this in the first pass of inlining because it looked so hard, but when I realized how many times it was called, and that I could inline just the beginning of the function to get more speed when the cache offset was active, I did it. The new macro formatting style is my idea too, and it makes things much simpler. Look at the ugly heap_getattr() macros in 6.2. -- Bruce Momjian | 830 Blythe Avenue maillist@candle.pha.pa.us | Drexel Hill, Pennsylvania 19026 + If your life is a hard drive, | (610) 353-9879(w) + Christ can be your backup. | (610) 853-3000(h)
> This is fine, and a good place to put it, though the port-specific > change should go AFTER the standard #define, not before it, so you do: > > #if !defined(SCO) > #define > #else > static ... > #endif I'll make this change before submitting the patch. > [...] The new macro formatting style is my > idea too, and it makes things much simpler. Look at the ugly > heap_getattr() macros in 6.2. Agreed, and I have. -- ____ | Billy G. Allie | Domain....: Bill.Allie@mug.org | /| | 7436 Hartwell | Compuserve: 76337,2061 |-/-|----- | Dearborn, MI 48126| MSN.......: B_G_Allie@email.msn.com |/ |LLIE | (313) 582-1540 |