bugCERTI - Bugs: bug #22798, tick(time, time) parameters ignored

 
 

bug #22798: tick(time, time) parameters ignored

Submitter:  Petr Gotthard <gotthardp>
Submitted:  Tue 01 Apr 2008 07:12:58 AM UTC
 
Category:  CERTI Severity:  * 4 - Important
Item Group:  HLA compliance error Status:  Ready For Test
Privacy:  Public Assigned to:  siron
Open/Closed:  Closed Release:  * 3.2.7
Reproducibility:  Every Time Fixed Release:  * Unknown
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       No canned response available

 

( Jump to the original submission )

Wed 23 Apr 2008 12:59:42 PM UTC, comment #11: 

Thanks Petr,
I've just applied the patch.

Eric NOULARD <erk>
Project Administrator
Wed 23 Apr 2008 12:19:59 PM UTC, comment #10: 

Thanks for applying the patch, but you used an old version of the patch (sorry for generating so many patches). I think the current CVS has two problems:
 1) several (many!) memory leaks due to missing delete.
 2) limited portability due to the isfinite function.

Attached is a patch (to the current CVS HEAD) that fixes both problems. (It's a cvs diff, so it should be applied directly in the RTIA directory.)

(file #15512)

Petr Gotthard <gotthardp>
Project Member
Mon 21 Apr 2008 07:47:16 AM UTC, comment #9: 

Change from Erk->Pierre.
Pierre is in charge of this now.

Eric NOULARD <erk>
Project Administrator
Tue 08 Apr 2008 09:09:09 AM UTC, comment #8: 

Petr,

I did review the patch for "performance penalty" point of view
as we are really concerned by making CERTI the fastest RTI
out there :-).

The main possible drawback of your patch is
the loop for timeout using gettimeofday (unix case).
gettimeofday is a system call which have its cost
may let you relieve processor in an unwanted manner.

I think we should accept the patch as is
(as soon as and if Pierre is OK) for now and then
tune it in order to find a more efficient "timeout" handling
technical solution.

I did began sort of "abstract Clock" class which may be used
for high perf clock operation.
Clock is an abstract class for which there is currently
2 sub-classes:
     PosixClock
     TSCClock
Those two works fine on linux and provides very-low overhead
"gettime"-like feature. TSCClock may be made portable on Windows
but I did not found the time to do it.

I'll write TODClock class using gettimeofday in order to have
at least one portable clock class and may be a specialized
Win32Clock which may use Win32 specific high perf timing
http://www.devsource.com/c/a/Techniques/High-Performance-Timing-under-Windows/

CERTI lib may then use a Clock* ClockFactory::getBestClock()
in order to get the best available Clock class on any platform.

This is a sidenote and not really a "must-have" for this patch
but I write it down here, and may open a task for this.

Eric NOULARD <erk>
Project Administrator
Thu 03 Apr 2008 07:24:40 AM UTC, comment #7: 

Thanks Petr,

The last patch does not contain conflicts any more.
Pierre would you review Petr last patch please
in order to see if it's fits with you previous
modification of blocking tick.

https://savannah.nongnu.org/bugs/download.php?file_id=15383

Eric NOULARD <erk>
Project Administrator
Wed 02 Apr 2008 04:30:44 PM UTC, comment #6: 

Attached is a second patch (unfortunatelly longer due to changed indentation). (The patch is against HEAD from March 30, 2008).

The core function __tick_kernel is made so generic that it can implement both HLA1.3 and IEEE1516 calls, invoking either a single- or multiple callbacks.
The TICK_REQUEST acknoweldegement is sent always, as it indicates return value (for tick(m,m) and IEEE versions).
The tick(MAXINT) is replaced by TICK(infinity), so now it has idenical behaviour, perhaps a little slower.

As an optimization we can create different TICK_XXXX messages to avoid float aritmetics where unnecessary, on the other making a separate variant for each tick() function may make the code pretty chaotic.

(file #15380)

Petr Gotthard <gotthardp>
Project Member
Wed 02 Apr 2008 01:18:47 PM UTC, comment #5: 

I don't have the HLA 1.3 specification, but in IEEE 1516.1 there is evokeCallback (10.37) and evokeMultipleCallbacks (10.38), as you've already found.
Additionaly, the HLA1.3 tick() syntax is described in SISO-STD-004-2004.

Yes, tick2() is not idential to tick(0.0, MAXINT), but I wanted to avoid creating yet another tick() implementation.

I've renamed tick_kernel --> __evoke_callback to highlight that it has the IEEE1516 semantics. I will change the name back because I changed the semantics due to the comment #2 problem.
The __prefix is meant to hightlight it's a non-standard extension.

The patch is made against HEAD from Sunday, because the CVS protocol doesn't pass Honeywell firewalls. I will create a new patch after solving the comment #2 problem.

Petr Gotthard <gotthardp>
Project Member
Wed 02 Apr 2008 01:05:12 PM UTC, comment #4: 

I did forget to tell that
in my RTI-NG doc the no-args tick() should
always return RTI::FALSE :-)

whereas tick(TickTime,TickTime)

should return RTI::TRUE if  processing was interrupt
by timeout
and RTI::FALSE otherwise

As of "evoke" Pierre did point me to
IEEE-1516-2001 document

10.37 Evoke Callback on page 208
I'll read it carefully and come back :-)

Eric NOULARD <erk>
Project Administrator
Wed 02 Apr 2008 12:51:50 PM UTC, comment #3: 

Hi Petr,

First of all thank you for the patch.
Here comes my opinion.
I add Pierre as CC since he recently did work
on tick/tick2/tick_kernel in order to avoid
CPU-consuming polling tick tight loop.

First of all as far as I know tick is not an
HLA-standardized API but was found in RTI and RTI-NG 1.3.
I cannot find any tick description in HLA 1.3 document
nor in IEEE Std 1516.1-2000.

The only tick doc I have is that RTI-NG RTIAmbassador
API description where it's written:
"This service does not correspond directly to a service specified in the HLA Interface Specification version 1.1 or version 1.3"

and

"This service is invoked by the federate to yield processor time to the LRC. During a tick() invocation, the LRC will process   incoming traffic, deliver callbacks to the federate, and perform
various internal RTI maintenance essential to the operation of the federation."

If I am wrong please give me the reference to tick in
IEEE-1516 documents.

Concerning tick2() it was introduced in order to avoid
polling tight loop which was just a pain in a HLA realtime
experiment. I don't think it's strictly equivalent
to tick(0.0,MAXINT) since tick2() is purely blocking
no-timeout at all. However tick(0.0,MAXINT) may do the job.
I let Pierre give his opinion on this.

Regarding the patch itself
Communications::readMessage patch is OK and straightforward
In Message* patch seems OK too even if we may
         create 2 different messages for avoiding the overhead
         of extra argument when "simple" tick is called.

In RTIAmbassador.cc
The tick_kernel --> __evoke_callback renaming does
not seem necesary to me? Where or why did you pick this name?

I have trouble applying you patch for
RTIA/RTIA.cc
RTIA/RTIA_federate.cc
can you tell what are the revision of those file
from which you built you patch?
And/or regenerate a patch against CERTI-3_2_6 or
current HEAD?

Eric NOULARD <erk>
Project Administrator
Wed 02 Apr 2008 11:17:38 AM UTC, comment #2: 

I think there's one bug more: the single-callback tick() always returns TRUE, but (at least IEEE1516 say that) the result should indicate pending callbacks.
The TICK_REQUEST acknowledgement should be sent after each TICK_REQUEST to deliver the result value to the federate.

Petr Gotthard <gotthardp>
Project Member
Tue 01 Apr 2008 05:23:47 PM UTC, comment #1: 

I'll review your patch soon.

Eric NOULARD <erk>
Project Administrator
Tue 01 Apr 2008 07:12:58 AM UTC, original submission:  

The tick(time, time) is defined, but the parameters are ignored, so the behaviour is identical to tick().
A non-standard tick2() function is provided, that has behaviour identical to tick(0,MAXINT).
I suggest to implement tick(time, time) and deprecate tick2().

I'm going to provide a patch for this issue later today.

Petr Gotthard <gotthardp>
Project Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #15512:  RTIA.patch added by gotthardp (3KiB - text/x-patch - Proposed fix.)
file #15383:  tick-3.patch added by gotthardp (41KiB - text/x-patch - diff against current CVS HEAD)
file #15380:  tick.patch.v2 added by gotthardp (40KiB - text/plain)
file #15373:  tick.patch added by gotthardp (12KiB - text/plain - Proposed patch.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by siron (Updated the item)
  • -email is unavailable- added by erk (As tick master)
  • -email is unavailable- added by erk (Posted a comment)
  • -email is unavailable- added by gotthardp (Submitted the item)
  •  

     

     

     

    Follow 12 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2008-04-26 erk Open/ClosedOpen Closed
        Release3.2.6 3.2.7
    2008-04-23 gotthardp Attached File- Added RTIA.patch, #15512
    2008-04-23 siron StatusIn Progress Ready For Test
    2008-04-21 erk Assigned toerk siron
    2008-04-02 gotthardp Attached File- Added tick-3.patch, #15383
    2008-04-02 gotthardp Attached File- Added tick.patch.v2, #15380
    2008-04-02 erk StatusNone In Progress
    2008-04-02 erk Carbon-Copy- Added siron
    2008-04-01 erk Assigned toNone erk
        ReleaseCVS 3.2.6
    2008-04-01 gotthardp Attached File- Added tick.patch, #15373

    Back to the top


    Powered by Savane 3.9