I'd like you to do a code review. To review this change, run gvn review --project https://v8.googlecode.com/svn/branches/bleeding_edge lrn/[EMAIL PROTECTED]
Alternatively, to review the latest snapshot of this change branch, run gvn --project https://v8.googlecode.com/svn/branches/bleeding_edge review lrn/lrn_relog to review the following change: *lrn/[EMAIL PROTECTED] | lrn | 2008-09-11 09:45:22 +-100 (Thu, 11 Sep 2008) Description: Added -log-regexp option to log all compilations and executions of regular expressions. Slightly modified SmartPointer. Made String.ToWideCString return a SmartPointer instead of a plain pointer. Affected Paths: M //branches/bleeding_edge/src/jsregexp.cc M //branches/bleeding_edge/src/log.cc M //branches/bleeding_edge/src/log.h M //branches/bleeding_edge/src/messages.cc M //branches/bleeding_edge/src/objects.cc M //branches/bleeding_edge/src/objects.h M //branches/bleeding_edge/src/smart-pointer.h This is a semiautomated message from "gvn mail". See <http://code.google.com/p/gvn/> to learn more. Index: src/jsregexp.cc =================================================================== --- src/jsregexp.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) +++ src/jsregexp.cc (^/changes/lrn/lrn_relog/bleeding_edge/src/[EMAIL PROTECTED]) @@ -205,6 +205,8 @@ Handle<Object> RegExpImpl::JsreCompile(Handle<JSVa value->set(INTERNAL_INDEX, *internal); re->set_value(*value); + LOG(RegExpCompileEvent(re)); + return re; } @@ -223,6 +225,8 @@ Handle<Object> RegExpImpl::JsreExecOnce(Handle<JSV const JSRegExp* js_regexp = reinterpret_cast<JSRegExp*>(internal->GetDataStartAddress()); + LOG(RegExpExecEvent(regexp, previous_index, subject)); + rc = jsRegExpExecute(js_regexp, two_byte_subject, subject->length(), previous_index, Index: src/log.cc =================================================================== --- src/log.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) +++ src/log.cc (^/changes/lrn/lrn_relog/bleeding_edge/src/[EMAIL PROTECTED]) @@ -52,6 +52,8 @@ DEFINE_bool(log_state_changes, false, "Log state c DEFINE_bool(log_suspect, false, "Log suspect operations."); DEFINE_bool(prof, false, "Log statistical profiling information (implies --log-code)."); +DEFINE_bool(log_regexp, false, + "Log regular expression execution."); DEFINE_bool(sliding_state_window, false, "Update sliding state window counters."); @@ -368,6 +370,76 @@ void Logger::SharedLibraryEvent(const wchar_t* lib #endif } +void Logger::LogRegExpSource(const Handle<JSValue> val) { + // Prints "/" + re.source + "/" + + // (re.global?"g":"") + (re.ignorecase?"i":"") + (re.multiline?"m":"") + + Handle<Object> source = GetProperty(val, "source"); + if (!source->IsString()) { + fprintf(logfile_, "no source"); + return; + } + Handle<String> sourceString = Handle<String>::cast(source); + + SmartPointer<uc16> cstringAlloc = sourceString->ToWideCString(); + fprintf(logfile_, "/"); + for(size_t i = 0, n = sourceString->length(); i < n; i++) { + uc16 c = cstringAlloc[i]; + if (c < 32 || (c > 126 && c <= 255)) { + fprintf(logfile_, "\\x%02x", c); + } else if (c > 255) { + fprintf(logfile_, "\\u%04x", c); + } else { + fprintf(logfile_, "%lc", c); + } + } + fprintf(logfile_, "/"); + + // global flag + Handle<Object> global = GetProperty(val, "global"); + if (global->IsTrue()) { + fprintf(logfile_, "g"); + } + // ignorecase flag + Handle<Object> ignorecase = GetProperty(val, "ignoreCase"); + if (ignorecase->IsTrue()) { + fprintf(logfile_, "i"); + } + // multiline flag + Handle<Object> multiline = GetProperty(val, "multiline"); + if (multiline->IsTrue()) { + fprintf(logfile_, "m"); + } +} + + +void Logger::RegExpCompileEvent(const Handle<JSValue> pattern) { +#ifdef ENABLE_LOGGING_AND_PROFILING + if (logfile_ == NULL || !FLAG_log_regexp) { return; } + ScopedLock sl(mutex_); + + fprintf(logfile_, "regexp-compile,"); + LogRegExpSource(pattern); + fprintf(logfile_, "\n"); + +#endif +} + + +void Logger::RegExpExecEvent(const Handle<JSValue> val, + int start_index, + Handle<String> string) { +#ifdef ENABLE_LOGGING_AND_PROFILING + if (logfile_ == NULL || !FLAG_log_regexp) { return; } + ScopedLock sl(mutex_); + + fprintf(logfile_, "regexp-run,"); + LogRegExpSource(val); + fprintf(logfile_, ",0x%08x,%d..%d\n", string->Hash(), start_index, string->length()); +#endif +} + + void Logger::ApiIndexedSecurityCheck(uint32_t index) { #ifdef ENABLE_LOGGING_AND_PROFILING if (logfile_ == NULL || !FLAG_log_api) return; @@ -612,6 +684,7 @@ bool Logger::Setup() { FLAG_log_gc = true; FLAG_log_suspect = true; FLAG_log_handles = true; + FLAG_log_regexp = true; } // --prof implies --log-code. @@ -620,7 +693,7 @@ bool Logger::Setup() { // Each of the individual log flags implies --log. Check after // checking --log-all and --prof in case they set --log-code. if (FLAG_log_api || FLAG_log_code || FLAG_log_gc || - FLAG_log_handles || FLAG_log_suspect) { + FLAG_log_handles || FLAG_log_suspect || FLAG_log_regexp) { FLAG_log = true; } Index: src/log.h =================================================================== --- src/log.h (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) +++ src/log.h (^/changes/lrn/lrn_relog/bleeding_edge/src/[EMAIL PROTECTED]) @@ -174,6 +174,14 @@ class Logger { unsigned start, unsigned end); + // ==== Events logged by --log-regexp ==== + // Regexp compilation and execution events. + + static void RegExpCompileEvent(const Handle<JSValue> regexp); + + static void RegExpExecEvent( + const Handle<JSValue> regexp, int start_index, Handle<String> string); + #ifdef ENABLE_LOGGING_AND_PROFILING static StateTag state() { return current_state_ ? current_state_->state() : OTHER; @@ -182,6 +190,10 @@ class Logger { #ifdef ENABLE_LOGGING_AND_PROFILING private: + + // Emits the source code of a regexp. Used by regexp events. + static void Logger::LogRegExpSource(const Handle<JSValue> regexp); + // Emits a profiler tick event. Used by the profiler thread. static void TickEvent(TickSample* sample, bool overflow); Index: src/messages.cc =================================================================== --- src/messages.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) +++ src/messages.cc (^/changes/lrn/lrn_relog/bleeding_edge/src/[EMAIL PROTECTED]) @@ -46,7 +46,7 @@ void MessageHandler::DefaultMessageReport(const Me } else { HandleScope scope; Handle<Object> data(loc->script()->name()); - SmartPointer<char> data_str = NULL; + SmartPointer<char> data_str; if (data->IsString()) data_str = Handle<String>::cast(data)->ToCString(DISALLOW_NULLS); PrintF("%s:%i: %s\n", *data_str ? *data_str : "<unknown>", Index: src/objects.cc =================================================================== --- src/objects.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) +++ src/objects.cc (^/changes/lrn/lrn_relog/bleeding_edge/src/[EMAIL PROTECTED]) @@ -3023,11 +3023,11 @@ const uc16* String::GetTwoByteData(unsigned start) } -uc16* String::ToWideCString(RobustnessFlag robust_flag) { +SmartPointer<uc16> String::ToWideCString(RobustnessFlag robust_flag) { ASSERT(NativeAllocationChecker::allocation_allowed()); if (robust_flag == ROBUST_STRING_TRAVERSAL && !LooksValid()) { - return NULL; + return SmartPointer<uc16>(); } Access<StringInputBuffer> buffer(&string_input_buffer); @@ -3041,7 +3041,7 @@ const uc16* String::GetTwoByteData(unsigned start) result[i++] = character; } result[i] = 0; - return result; + return SmartPointer<uc16>(result); } Index: src/objects.h =================================================================== --- src/objects.h (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) +++ src/objects.h (^/changes/lrn/lrn_relog/bleeding_edge/src/[EMAIL PROTECTED]) @@ -2864,7 +2864,7 @@ class String: public HeapObject { // ROBUST_STRING_TRAVERSAL invokes behaviour that is robust This means it // handles unexpected data without causing assert failures and it does not // do any heap allocations. This is useful when printing stack traces. - uc16* ToWideCString(RobustnessFlag robustness_flag = FAST_STRING_TRAVERSAL); + SmartPointer<uc16> ToWideCString(RobustnessFlag robustness_flag = FAST_STRING_TRAVERSAL); // Tells whether the hash code has been computed. inline bool HasHashCode(); Index: src/smart-pointer.h =================================================================== --- src/smart-pointer.h (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) +++ src/smart-pointer.h (^/changes/lrn/lrn_relog/bleeding_edge/src/[EMAIL PROTECTED]) @@ -31,21 +31,20 @@ namespace v8 { namespace internal { -// A 'scoped pointer' that calls delete[] on its pointer when the +// A 'scoped array pointer' that calls DeleteArray on its pointer when the // destructor is called. template<typename T> class SmartPointer { public: - // Construct a scoped pointer from a plain one. - inline SmartPointer(T* pointer) : p(pointer) {} + // Default constructor. Construct an empty scoped pointer. + inline SmartPointer() : p(NULL) {} - // When the destructor of the scoped pointer is executed the plain pointer - // is deleted using DeleteArray. This implies that you must allocate with - // NewArray. - inline ~SmartPointer() { if (p) DeleteArray(p); } + // Construct a scoped pointer from a plain one. + explicit inline SmartPointer(T* pointer) : p(pointer) {} + // Copy constructor removes the pointer from the original to avoid double // freeing. inline SmartPointer(const SmartPointer<T>& rhs) : p(rhs.p) { @@ -53,14 +52,21 @@ class SmartPointer { } + // When the destructor of the scoped pointer is executed the plain pointer + // is deleted using DeleteArray. This implies that you must allocate with + // NewArray. + inline ~SmartPointer() { if (p) DeleteArray(p); } + + // You can get the underlying pointer out with the * operator. inline T* operator*() { return p; } - // You can use -> as if it was a plain pointer. - inline T* operator->() { return p; } + // You can use [n] to index as if it was a plain pointer + inline T& operator[](size_t i) { + return p[i]; + } - // We don't have implicit conversion to a T* since that hinders migration: // You would not be able to change a method from returning a T* to // returning an SmartPointer<T> and then get errors wherever it is used. @@ -81,9 +87,10 @@ class SmartPointer { // double freeing. inline SmartPointer& operator=(const SmartPointer<T>& rhs) { ASSERT(p == NULL); - p = rhs.p; - const_cast<SmartPointer<T>&>(rhs).p = NULL; - return *this; + T* tmp = rhs.p; // swap to handle self-assignment + const_cast<SmartPointer<T>&>(rhs).p = NULL; + p = tmp; + return *this; } --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
