[This commit e-mail is a repeat.]
#At file:///home/mysql_src/bzrrepos_new/mysql-next-mr-opt-backporting-wl4800-new2/ based
on revid:jorgen.loland@stripped
3290 Guilhem Bichot 2011-04-12
Proposal suggested by Jorgen: move content of Struct_desc into Opt_trace_struct.
Opt_trace_stmt functions get the information which they need
through new parameters to open_struct()/close_struct().
There is only one "encapsulation breakage" (one new public setter), it's when
Opt_trace_stmt::separator() has to inform Opt_trace_struct that it's not empty
anymore. But this is minor and I could not find a way to clean this up.
Partial_access_for_Opt_trace_stmt is deleted.
modified:
sql/opt_trace.cc
sql/opt_trace.h
=== modified file 'sql/opt_trace.cc'
--- a/sql/opt_trace.cc 2011-04-05 07:34:13 +0000
+++ b/sql/opt_trace.cc 2011-04-12 13:40:12 +0000
@@ -75,15 +75,18 @@ public:
may have been written to, will likely be invalid JSON.
*/
bool open_struct(const char *key, Opt_trace_struct *ots,
- Opt_trace_context::feature_value feature,
- char opening_bracket);
+ bool has_disabled_I_S, char opening_bracket);
/**
- When closing an Opt_trace_struct: adds the closing bracket and optionally
- the key to the trace buffer, updates current_struct.
+ When closing an Opt_trace_struct:
+ - adds the closing bracket and optionally the key to the trace buffer
+ - re-enables I_S output if the dying structure had disabled it
+ - updates current_struct.
@param saved_key key or NULL
+ @param has_disabled_I_S whether structure had disabled I_S output
@param closing_bracket closing bracket to use
*/
- void close_struct(const char *saved_key, char closing_bracket);
+ void close_struct(const char *saved_key, bool has_disabled_I_S,
+ char closing_bracket);
/// Put optional comma, newline and indentation
void separator();
@@ -167,28 +170,8 @@ private:
Opt_trace_context *ctx; ///< context
Opt_trace_struct *current_struct; ///< current open structure
- /**
- Whether we added data (scalars, structures) to the current struct.
- This is used and maintained only if sending to I_S.
- */
- bool current_struct_empty;
- /// @sa stack_of_current_structs
- struct Struct_desc
- {
- Opt_trace_struct *current_struct;
- bool has_disabled_I_S;
- bool current_struct_empty;
- };
- /**
- Same logic as Opt_trace_context::stack_of_current_stmts. We need to
- remember:
- - each value of current_struct
- - whether such structure caused tracing to be disabled in this statement
- (happens when the structure belongs to a not-traced optimizer feature,
- in accordance with the value of @@@@optimizer_trace_features)
- - whether such structure was empty.
- */
- Dynamic_array<Struct_desc> stack_of_current_structs;
+ /// Same logic as Opt_trace_context::stack_of_current_stmts.
+ Dynamic_array<Opt_trace_struct *> stack_of_current_structs;
/**
When we temporarily disable I_S (because of Opt_trace_disable_I_S, or
@@ -287,29 +270,6 @@ private:
};
-/**
- This is a trick.
- The problem is that Opt_trace_stmt needs access to
- Opt_trace_struct::check_key(). We don't want to make Opt_trace_stmt a
- friend of Opt_trace_struct, that would be giving Opt_trace_stmt access to
- too many things. So we have to make check_key() public. This isn't good
- either: function becomes accessible to includers of opt_trace.h.
- Instead, we declare the class below as friend of Opt_trace_struct; this
- class only calls check_key(), and Opt_trace_stmt is a friend of it. The
- idea is that, of Opt_trace_struct, only check_key() is made accessible
- to Opt_trace_stmt, and only to it, via the class below.
- Let me know what option to choose, trick or public.
-*/
-class Partial_access_for_Opt_trace_stmt
-{
-private:
- static const char *check_key(Opt_trace_struct *s, const char *key)
- { return s->check_key(key); }
-
- friend class Opt_trace_stmt;
-};
-
-
// implementation of class Opt_trace_struct
namespace {
@@ -339,7 +299,9 @@ void Opt_trace_struct::do_construct(Opt_
#ifndef DBUG_OFF
previous_key[0]= 0;
#endif
- if (likely(!stmt->open_struct(key, this, feature,
+ has_disabled_I_S= !ctx->feature_enabled(feature);
+ empty= true;
+ if (likely(!stmt->open_struct(key, this, has_disabled_I_S,
opening_bracket(requires_key))))
started= true;
}
@@ -349,7 +311,8 @@ void Opt_trace_struct::do_destruct()
{
DBUG_PRINT("opt", ("%s: ending struct", saved_key));
DBUG_ASSERT(started);
- stmt->close_struct(saved_key, closing_bracket(requires_key));
+ stmt->close_struct(saved_key, has_disabled_I_S,
+ closing_bracket(requires_key));
started= false;
}
@@ -561,13 +524,12 @@ void Opt_trace_stmt::set_query(const cha
bool Opt_trace_stmt::open_struct(const char *key, Opt_trace_struct *ots,
- Opt_trace_context::feature_value feature,
+ bool has_disabled_I_S,
char opening_bracket)
{
- bool has_disabled_I_S= false;
if (support_I_S == YES_FOR_THIS)
{
- if (!ctx->feature_enabled(feature))
+ if (has_disabled_I_S)
{
/*
@@ -589,13 +551,11 @@ bool Opt_trace_stmt::open_struct(const c
if (unlikely(stack_of_values_of_support_I_S.append(support_I_S)))
goto err;
support_I_S= NO_FOR_THIS_AND_CHILDREN;
- has_disabled_I_S= true;
}
else
{
if (current_struct != NULL)
- key= Partial_access_for_Opt_trace_stmt::
- check_key(current_struct, key);
+ key= current_struct->check_key(key);
trace_buffer.prealloc();
separator();
if (key != NULL)
@@ -608,10 +568,9 @@ bool Opt_trace_stmt::open_struct(const c
}
}
{
- Struct_desc d= { current_struct, has_disabled_I_S, current_struct_empty};
DBUG_EXECUTE_IF("opt_trace_oom_in_open_struct",
DBUG_SET("+d,simulate_out_of_memory"););
- const bool rc= stack_of_current_structs.append(d);
+ const bool rc= stack_of_current_structs.append(current_struct);
/*
If the append() above didn't trigger reallocation, we need to turn the
symbol off by ourselves, or it could make an unrelated allocation
@@ -623,7 +582,6 @@ bool Opt_trace_stmt::open_struct(const c
goto err;
}
current_struct= ots;
- current_struct_empty= true;
return false;
err:
return true;
@@ -631,12 +589,10 @@ err:
void Opt_trace_stmt::close_struct(const char *saved_key,
+ bool has_disabled_I_S,
char closing_bracket)
{
- Struct_desc d= stack_of_current_structs.pop();
- current_struct= d.current_struct;
- current_struct_empty= d.current_struct_empty;
-
+ current_struct= stack_of_current_structs.pop();
if (support_I_S == YES_FOR_THIS)
{
next_line();
@@ -648,7 +604,7 @@ void Opt_trace_stmt::close_struct(const
trace_buffer.append(STRING_WITH_LEN(" */"));
}
}
- if (d.has_disabled_I_S)
+ if (has_disabled_I_S)
support_I_S= stack_of_values_of_support_I_S.pop();
}
@@ -659,10 +615,9 @@ void Opt_trace_stmt::separator()
// Put a comma first, if we have already written an object at this level.
if (current_struct != NULL)
{
- if (!current_struct_empty)
+ if (!current_struct->set_not_empty())
trace_buffer.append(',');
next_line();
- current_struct_empty= false;
}
}
=== modified file 'sql/opt_trace.h'
--- a/sql/opt_trace.h 2011-04-05 07:34:13 +0000
+++ b/sql/opt_trace.h 2011-04-12 13:40:12 +0000
@@ -716,8 +716,6 @@ private:
};
-class Partial_access_for_Opt_trace_stmt;
-
/**
Object and array are both "structured data" and have lots in common, so the
Opt_trace_struct is a base class for them.
@@ -982,6 +980,25 @@ public:
*/
void end() { if (unlikely(started)) do_destruct(); }
+ /**
+ Informs this structure that we are adding data (scalars, structures) to
+ it.
+ This is used only if sending to I_S.
+ @returns whether the structure was empty so far.
+ @note this is reserved for use by Opt_trace_stmt.
+ */
+ bool set_not_empty()
+ {
+ const bool old_empty= empty;
+ empty= false;
+ return old_empty;
+ }
+ /**
+ Validates the key about to be added.
+ @note this is reserved for use by Opt_trace_stmt.
+ */
+ const char *check_key(const char *key);
+
private:
/// Full initialization. @sa Opt_trace_struct::Opt_trace_struct
@@ -1006,9 +1023,6 @@ private:
Opt_trace_struct& do_add_null(const char *key);
Opt_trace_struct& do_add_utf8_table(TABLE *tab);
- const char *check_key(const char *key); ///< Validates the key
- friend class Partial_access_for_Opt_trace_stmt;
-
Opt_trace_struct(const Opt_trace_struct&); ///< not defined
Opt_trace_struct& operator=(const Opt_trace_struct&); ///< not defined
@@ -1038,6 +1052,14 @@ private:
*/
char previous_key[25];
#endif
+
+ /**
+ Whether this structure caused tracing to be disabled in this statement
+ because belonging to a not-traced optimizer feature, in accordance with
+ the value of @@@@optimizer_trace_features.
+ */
+ bool has_disabled_I_S;
+ bool empty; ///< @see is_empty()
};
Attachment: [text/bzr-bundle] bzr/guilhem.bichot@oracle.com-20110412134012-4yv0l797yoz8qynu.bundle
