[m-dev.] For review: calling MR_collect_filter() in MR_trace_real()

Erwan Jahier Erwan.Jahier at irisa.fr
Tue Mar 6 19:54:44 AEDT 2001


Fergus wrote :

| I like the general direction of this change.  Handling collect in
| MR_trace_real() is much better.
| 
| However, I would like to keep the implementation details of the external
| tracer, in particular the calls to do socket I/O, out of mercury_trace.h
| and mercury_trace.c.
| 
| So how about making the following changes:
|
| ...

Your proposal works fine. Thanks. I give the new diff below.

| 
| > +				MR_trace_ctrl.MR_trace_cmd = MR_CMD_GOTO;
| > +				MR_copy_saved_regs_to_regs(event_info.MR_max_mr_num, 
| > +						saved_regs);
| > +				return MR_trace_event(&MR_trace_ctrl, TRUE,
| > +                                               layout, port, seqno, depth);
| > +			}
| > +			MR_copy_saved_regs_to_regs(event_info.MR_max_mr_num, 
| > +				saved_regs);
| 
| The call to MR_copy_saved_regs_to_regs() is duplicated
| in both the `stop_collecting' and the `!stop_collecting'
| cases; can it be moved to before the `if (stop_collecting)'?

Now that MR_send_collect_result() is called from mercury_trace_external(), yes.

| On 05-Mar-2001, Erwan Jahier <Erwan.Jahier at irisa.fr> wrote:
| > +++ trace/mercury_trace_external.c	2001/03/05 17:30:14
| > @@ -135,6 +134,15 @@
| >  static	void	(*send_collect_result_ptr)(MR_Word, MR_Word);
| >  
| >  /*
| > +** Function pointer which is applied to each event during collect requests.
| > +*/
| > +
| > +static void	(*filter_ptr)(MR_Integer, MR_Integer, MR_Integer, MR_Word,
| > +			MR_Word, MR_String, MR_String, MR_String, MR_Integer,
| > +			MR_Integer, MR_Word, MR_Integer, MR_String, MR_Word, 
| > +			MR_Word *, MR_Char *);
| 
| I think it would be nicer to make that a field in the MR_Trace_Cmd_Info
| (and passing this field to MR_COLLECT_filter(), as before),
| rather than making it a static variable.
| 
| Static variables cause problems for reentrancy, which causes various
| difficulties.

Ok.

| > @@ -1504,7 +1489,7 @@
| >  			arguments = MR_list_empty()
| >  		);
| >  	}
| > -	MR_TRACE_CALL_MERCURY((*filter_ptr)(
| > +	(*filter_ptr)(
| >  		MR_trace_event_number,
| >  		seqno,
| >  		depth,
| 
| What happened to the MR_TRACE_CALL_MERCURY() macro here?
| Why did that go away?

Because I think they are unnecessary, indeed: 
MR_copy_regs_to_saved_regs() calls 
	restore_transient_registers();
	save_registers();

MR_copy_saved_regs_to_regs() calls
	restore_registers();
	save_transient_registers();

Since MR_COLLECT_filter() is called just between MR_copy_regs_to_saved_regs()
and MR_copy_saved_regs_to_regs(), I thought the macro MR_TRACE_CALL_MERCURY()
was not necessary in that case. 

****************************************************************************
Here are the new log message and the new diff:

--
Estimated hours taken: 12
branches: main and release.

Handling collect requests in MR_trace_real() rather than in
MR_trace_event_external() as it saves a few indirections and a few
function calls. It matters since this code is typically called
several million of times per seconds. 

trace/mercury_trace_external.c:
trace/mercury_trace.[ch]:
	Handling collect requests in MR_trace_real() rather than in
	MR_trace_event_external(). Add the MR_trace_cmd mode `MR_CMD_COLLECT' 
	in mercury_trace.c. 

	Add the field `MR_filter_ptr' to MR_Trace_Cmd_Info that points to the 
	collecting filtering procedure. Use that field instead of *filter_ptr.

trace/mercury_trace_external.h:
	Make MR_collect_filter() an extern function to be able to use it 
	from mercury_trace.c.


Index: trace/mercury_trace.c
===================================================================
RCS file: /home/mercury1/repository/mercury/trace/mercury_trace.c,v
retrieving revision 1.39
diff -u -d -u -r1.39 mercury_trace.c
--- trace/mercury_trace.c	2001/03/05 03:52:29	1.39
+++ trace/mercury_trace.c	2001/03/06 08:53:01
@@ -57,7 +57,8 @@
 	0,	/* stop event */
 	MR_PRINT_LEVEL_SOME,
 	FALSE,	/* not strict */
-	TRUE	/* must check */
+	TRUE,	/* must check */
+	NULL
 };
 
 MR_Code 		*MR_trace_real(const MR_Label_Layout *layout);
@@ -166,6 +167,44 @@
 #endif	/* MR_TRACE_HISTOGRAM */
 
 	switch (MR_trace_ctrl.MR_trace_cmd) {
+		case MR_CMD_COLLECT:
+		  {
+		        MR_Event_Info	event_info;
+			Word		*saved_regs = event_info.MR_saved_regs;
+			int		max_r_num;
+			const char	*path;
+			bool    	stop_collecting = FALSE;
+
+			max_r_num = layout->MR_sll_entry->MR_sle_max_r_num;
+			if (max_r_num + MR_NUM_SPECIAL_REG > 
+					MR_MAX_SPECIAL_REG_MR) 
+			{
+				event_info.MR_max_mr_num = 
+					max_r_num + MR_NUM_SPECIAL_REG;
+			} else {
+				event_info.MR_max_mr_num = 
+					MR_MAX_SPECIAL_REG_MR;
+			}
+			
+			port = (MR_Trace_Port) layout->MR_sll_port;
+			path = layout->MR_sll_entry->MR_sle_module_layout
+				->MR_ml_string_table + layout->MR_sll_goal_path;
+			MR_copy_regs_to_saved_regs(event_info.MR_max_mr_num, 
+				saved_regs);
+			MR_trace_init_point_vars(layout, saved_regs, port);
+			MR_COLLECT_filter(MR_trace_ctrl.MR_filter_ptr, seqno,
+				depth, port, layout, path, &stop_collecting);
+			MR_copy_saved_regs_to_regs(event_info.MR_max_mr_num, 
+				saved_regs);
+			if (stop_collecting) {
+				MR_trace_ctrl.MR_trace_cmd = MR_CMD_GOTO;
+				return MR_trace_event(&MR_trace_ctrl, TRUE,
+                                               layout, port, seqno, depth);
+			}
+
+			goto check_stop_print;
+		  }	
+
 		case MR_CMD_GOTO:
 			if (MR_trace_event_number >=
 					MR_trace_ctrl.MR_trace_stop_event)
Index: trace/mercury_trace.h
===================================================================
RCS file: /home/mercury1/repository/mercury/trace/mercury_trace.h,v
retrieving revision 1.21
diff -u -d -u -r1.21 mercury_trace.h
--- trace/mercury_trace.h	2001/02/13 08:28:27	1.21
+++ trace/mercury_trace.h	2001/03/06 08:53:01
@@ -129,6 +129,10 @@
 ** MR_trace_cmd says what mode the tracer is in, i.e. how events should be
 ** treated.
 **
+** If MR_trace_cmd == MR_CMD_COLLECT, the event handler calls MR_COLLECT_filter()
+** until the end of the execution is reached or until the `stop_collecting'
+** variable is set to TRUE. It is the tracer mode after a `collect' request.
+**
 ** If MR_trace_cmd == MR_CMD_GOTO, the event handler will stop at the next
 ** event whose event number is equal to or greater than MR_trace_stop_event.
 **
@@ -162,6 +166,7 @@
 */
 
 typedef enum {
+	MR_CMD_COLLECT,
 	MR_CMD_GOTO,
 	MR_CMD_NEXT,
 	MR_CMD_FINISH,
@@ -202,6 +207,12 @@
 				** MR_trace_print_level != MR_PRINT_LEVEL_NONE
 				*/
 	bool			MR_trace_must_check;
+
+				/*
+				** The MR_filter_ptr field points to the filter/4
+				** procedure during a collect request
+				*/
+	void *    		MR_filter_ptr;
 } MR_Trace_Cmd_Info;
 
 #define	MR_port_is_final(port)		((port) == MR_PORT_EXIT || \
Index: trace/mercury_trace_external.c
===================================================================
RCS file: /home/mercury1/repository/mercury/trace/mercury_trace_external.c,v
retrieving revision 1.53
diff -u -d -u -r1.53 mercury_trace_external.c
--- trace/mercury_trace_external.c	2001/01/18 01:19:15	1.53
+++ trace/mercury_trace_external.c	2001/03/06 08:53:01
@@ -203,14 +203,6 @@
 static void	MR_get_variable_name(MR_Word debugger_request,
 			MR_String *var_name_ptr);
 static void	MR_trace_browse_one_external(MR_Var_Spec which_var);
-static void	MR_COLLECT_filter(void (*filter_ptr)(MR_Integer, MR_Integer,
-			MR_Integer, MR_Word, MR_Word, MR_String, MR_String,
-			MR_String, MR_Integer, MR_Integer, MR_Word, MR_Integer,
-			MR_String, MR_Word, MR_Word *, MR_Char *),
-			MR_Unsigned seqno, MR_Unsigned depth,
-			MR_Trace_Port port, 
-			const MR_Label_Layout *layout, const char *path, 
-			bool *stop_collecting);
 static void	MR_send_collect_result(void);
 
 #if 0
@@ -477,17 +469,11 @@
 MR_Code *
 MR_trace_event_external(MR_Trace_Cmd_Info *cmd, MR_Event_Info *event_info)
 {
-	static	MR_Word		search_data;
-	static	void		(*initialize_ptr)(MR_Word *);
-	static	void    	(*filter_ptr)(MR_Integer, MR_Integer,
-					MR_Integer, MR_Word, MR_Word,
-					MR_String, MR_String, MR_String,
-					MR_Integer, MR_Integer, MR_Word,
-					MR_Integer, MR_String, MR_Word,
-					MR_Word *, MR_Char *);
-	static	void		(*get_collect_var_type_ptr)(MR_Word *);
-	static	bool    	collect_linked = FALSE;
-	bool    		stop_collecting = FALSE;
+	static MR_Word	search_data;
+	static void	(*initialize_ptr)(MR_Word *);
+	static void	(*get_collect_var_type_ptr)(MR_Word *);
+	static bool    	collect_linked = FALSE;
+	bool    	stop_collecting = FALSE;
 	MR_Integer		debugger_request_type;
 	MR_Word			debugger_request;
 	MR_Word			var_list;
@@ -544,17 +530,10 @@
 			break;
 
 		case MR_collecting:
-		 
-			MR_COLLECT_filter(*filter_ptr, seqno, depth, port,
-				layout, path, &stop_collecting);
+		        MR_send_collect_result(); 
+			MR_send_message_to_socket("execution_continuing");
+			break;
 
-			if (stop_collecting) {
-				MR_send_collect_result();
-				MR_send_message_to_socket("execution_continuing");
-				break;
-			} else {
-				goto done;
-			}
 		case MR_reading_request:
 			break;
 
@@ -784,7 +763,7 @@
 				MR_TRACE_CALL_MERCURY(
 					ML_CL_link_collect(
 			       		    MR_object_file_name,
-					    (MR_Word *) &filter_ptr,
+					    (MR_Word *) &cmd->MR_filter_ptr,
 					    (MR_Word *) &initialize_ptr,
 					    (MR_Word *) &send_collect_result_ptr,
 					    (MR_Word *) &get_collect_var_type_ptr,
@@ -827,8 +806,9 @@
 					** the current event, we need to call 
 					** filter once here.
 					*/
-					MR_COLLECT_filter(*filter_ptr, seqno, depth,
-						port, layout, path, &stop_collecting);
+					MR_COLLECT_filter(cmd->MR_filter_ptr,
+						seqno, depth, port, layout, path, 
+						&stop_collecting);
 					
 					if (stop_collecting) {
 						MR_send_collect_result();
@@ -836,6 +816,16 @@
 							"execution_continuing");
 						break;
 					} else {
+					/*
+					** For efficiency, the remaining calls
+					** to MR_COLLECT_filter() are done in 
+					** MR_trace_real().
+					*/
+					        cmd->MR_trace_cmd = MR_CMD_COLLECT;
+						cmd->MR_trace_must_check = FALSE;
+						cmd->MR_trace_strict = TRUE;
+						cmd->MR_trace_print_level = 
+						 	MR_PRINT_LEVEL_NONE;
 						goto done;
 					}
 				} else {
@@ -1477,13 +1467,12 @@
 ** This function calls the collect filtering predicate defined by the user
 ** and dynamically link with the execution.
 */
-static void
-MR_COLLECT_filter(void (*filter_ptr)(MR_Integer, MR_Integer, MR_Integer,
-	MR_Word, MR_Word, MR_String, MR_String, MR_String, MR_Integer,
-	MR_Integer, MR_Word, MR_Integer, MR_String, MR_Word, MR_Word *,
-	MR_Char *), MR_Unsigned seqno, MR_Unsigned depth, MR_Trace_Port port, 
-	const MR_Label_Layout *layout, const char *path, 
-	bool *stop_collecting)
+void
+MR_COLLECT_filter(void (*filter_ptr)(MR_Integer, MR_Integer, MR_Integer, MR_Word, 
+	MR_Word, MR_String, MR_String, MR_String, MR_Integer, MR_Integer, MR_Word, 
+	MR_Integer, MR_String, MR_Word, MR_Word *, MR_Char *), 
+	MR_Unsigned seqno, MR_Unsigned depth, MR_Trace_Port port, 
+	const MR_Label_Layout *layout, const char *path, bool *stop_collecting)
 {
 	MR_Char	result;		
 	MR_Word	arguments;
@@ -1504,7 +1493,7 @@
 			arguments = MR_list_empty()
 		);
 	}
-	MR_TRACE_CALL_MERCURY((*filter_ptr)(
+	(*filter_ptr)(
 		MR_trace_event_number,
 		seqno,
 		depth,
@@ -1520,7 +1509,7 @@
 		(MR_String) path,
 		MR_collecting_variable,
 		&MR_collecting_variable,
-		&result));
+		&result);
 	*stop_collecting = (result == 'y');
 }
 
Index: trace/mercury_trace_external.h
===================================================================
RCS file: /home/mercury1/repository/mercury/trace/mercury_trace_external.h,v
retrieving revision 1.9
diff -u -d -u -r1.9 mercury_trace_external.h
--- trace/mercury_trace_external.h	2001/02/13 08:28:27	1.9
+++ trace/mercury_trace_external.h	2001/03/06 08:53:01
@@ -18,6 +18,13 @@
 extern	void	MR_trace_final_external(void);
 extern	MR_Code	*MR_trace_event_external(MR_Trace_Cmd_Info *cmd,
 			MR_Event_Info *event_info);
+extern void	MR_COLLECT_filter(void (*filter_ptr)(MR_Integer, MR_Integer,
+			MR_Integer, MR_Word, MR_Word, MR_String, MR_String,
+			MR_String, MR_Integer, MR_Integer, MR_Word, MR_Integer,
+			MR_String, MR_Word, MR_Word *, MR_Char *), MR_Unsigned seqno, 
+			MR_Unsigned depth, MR_Trace_Port port, 
+			const MR_Label_Layout *layout, const char *path, 
+			bool *stop_collecting);
 
 /*
 ** External debugger socket streams.



-- 
R1.


--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------



More information about the developers mailing list