From 403368fb92b112dcf95ca1c5ee7d0eae8b1124f3 Mon Sep 17 00:00:00 2001 From: Eugene Lazutkin Date: Wed, 5 Jun 2024 23:34:30 -0500 Subject: [PATCH] Made the cache of the last string a weak pointer. Refs #211. --- lib/accessors.cc | 28 -------------------- lib/addon.cc | 67 +++++++++++++++++++++++------------------------ lib/new.cc | 7 +---- lib/str-val.h | 1 + lib/to_string.cc | 4 --- lib/util.h | 5 +--- lib/wrapped_re2.h | 30 +++------------------ re2.js | 2 +- 8 files changed, 40 insertions(+), 104 deletions(-) diff --git a/lib/accessors.cc b/lib/accessors.cc index 77c4984..47a22a8 100644 --- a/lib/accessors.cc +++ b/lib/accessors.cc @@ -28,30 +28,6 @@ NAN_GETTER(WrappedRE2::GetInternalSource) info.GetReturnValue().Set(Nan::New(re2->regexp.pattern()).ToLocalChecked()); } -NAN_GETTER(WrappedRE2::GetEnabledCache) -{ - if (!WrappedRE2::HasInstance(info.This())) - { - info.GetReturnValue().SetUndefined(); - return; - } - - auto re2 = Nan::ObjectWrap::Unwrap(info.This()); - info.GetReturnValue().Set(re2->enabledCache); -} - -NAN_GETTER(WrappedRE2::GetIsCached) -{ - if (!WrappedRE2::HasInstance(info.This())) - { - info.GetReturnValue().SetUndefined(); - return; - } - - auto re2 = Nan::ObjectWrap::Unwrap(info.This()); - info.GetReturnValue().Set(!!re2->lastStringValue); -} - NAN_GETTER(WrappedRE2::GetFlags) { if (!WrappedRE2::HasInstance(info.This())) @@ -63,10 +39,6 @@ NAN_GETTER(WrappedRE2::GetFlags) auto re2 = Nan::ObjectWrap::Unwrap(info.This()); std::string flags; - if (re2->enabledCache) - { - flags += "\b"; - } if (re2->hasIndices) { flags += "d"; diff --git a/lib/addon.cc b/lib/addon.cc index d446385..88b78c5 100644 --- a/lib/addon.cc +++ b/lib/addon.cc @@ -74,8 +74,6 @@ v8::Local WrappedRE2::Init() Nan::SetAccessor(instanceTemplate, Nan::New("hasIndices").ToLocalChecked(), GetHasIndices); Nan::SetAccessor(instanceTemplate, Nan::New("lastIndex").ToLocalChecked(), GetLastIndex, SetLastIndex); Nan::SetAccessor(instanceTemplate, Nan::New("internalSource").ToLocalChecked(), GetInternalSource); - Nan::SetAccessor(instanceTemplate, Nan::New("enabledCache").ToLocalChecked(), GetEnabledCache); - Nan::SetAccessor(instanceTemplate, Nan::New("isCached").ToLocalChecked(), GetIsCached); auto ctr = Nan::GetFunction(tpl).ToLocalChecked(); @@ -96,46 +94,31 @@ NODE_MODULE_INIT() WrappedRE2::~WrappedRE2() { - for (auto ptr : callbackRegistry) - { - *ptr = nullptr; - } dropLastString(); } // private methods -WrappedRE2::PtrWrappedRE2 *WrappedRE2::registerCallback() -{ - PtrWrappedRE2 *ptr = new PtrWrappedRE2(this); - callbackRegistry.insert(ptr); - return ptr; -} - -void WrappedRE2::unregisterCallback(PtrWrappedRE2 *ptr) -{ - callbackRegistry.erase(ptr); -} - void WrappedRE2::dropLastString() { - lastString.Reset(); - if (lastStringValue) + if (!lastString.IsEmpty()) { - delete lastStringValue; - lastStringValue = nullptr; + // lastString.ClearWeak(); + lastString.Reset(); } + if (!lastCache.IsEmpty()) + { + // lastCache.ClearWeak(); + lastCache.Reset(); + } + lastStringValue = nullptr; } -void WrappedRE2::weakLastStringCallback(const Nan::WeakCallbackInfo &data) +static void weakLastCacheCallback(const Nan::WeakCallbackInfo &data) { - PtrWrappedRE2 *re2 = data.GetParameter(); - if (*re2) - { - (*re2)->unregisterCallback(re2); - (*re2)->dropLastString(); - } - delete re2; + StrValBase *lastStringValue = data.GetParameter(); + Nan::AdjustExternalMemory(-(long)(lastStringValue->size)); + delete lastStringValue; } void WrappedRE2::prepareLastString(const v8::Local &arg, bool ignoreLastIndex) @@ -152,21 +135,37 @@ void WrappedRE2::prepareLastString(const v8::Local &arg, bool ignoreL // String // check if the same string is already in the cache - if (lastString == arg && lastStringValue) + if (lastString == arg && !lastCache.IsEmpty()) { if (!global && !sticky) return; // we are good + lastStringValue = static_cast(Nan::GetInternalFieldPointer(Nan::New(lastCache), 0)); lastStringValue->setIndex(startFrom); return; } dropLastString(); + // cache the string + lastStringValue = new StrValString(arg, startFrom); + if (lastStringValue->isBad) return; + Nan::AdjustExternalMemory(lastStringValue->size); + + // keep a weak pointer to the string lastString.Reset(arg); static_cast &>(lastString).SetWeak(); - Nan::Persistent dummy(arg); - dummy.SetWeak(registerCallback(), weakLastStringCallback, Nan::WeakCallbackType::kParameter); + // create a holder object for the cache + v8::Local objectTemplate = Nan::New(); + objectTemplate->SetInternalFieldCount(1); + v8::Local object = Nan::NewInstance(objectTemplate).ToLocalChecked(); + Nan::SetInternalFieldPointer(object, 0, lastStringValue); - lastStringValue = new StrValString(arg, startFrom); + // invalidate the cache if the holder object is garbage collected + Nan::Persistent placeHolderForCache(object); + placeHolderForCache.SetWeak(lastStringValue, weakLastCacheCallback, Nan::WeakCallbackType::kParameter); + + // keep a weak pointer to the cache + lastCache.Reset(object); + static_cast &>(lastCache).SetWeak(); }; diff --git a/lib/new.cc b/lib/new.cc index b0d2d8e..e82cc05 100644 --- a/lib/new.cc +++ b/lib/new.cc @@ -233,7 +233,6 @@ NAN_METHOD(WrappedRE2::New) bool unicode = false; bool sticky = false; bool hasIndices = false; - bool enabledCache = false; auto context = Nan::GetCurrentContext(); bool needFlags = true; @@ -257,9 +256,6 @@ NAN_METHOD(WrappedRE2::New) { switch (data[i]) { - case '\b': - enabledCache = true; - break; case 'g': global = true; break; @@ -343,7 +339,6 @@ NAN_METHOD(WrappedRE2::New) if (needFlags) { - enabledCache = re2->enabledCache; global = re2->global; ignoreCase = re2->ignoreCase; multiline = re2->multiline; @@ -406,7 +401,7 @@ NAN_METHOD(WrappedRE2::New) options.set_dot_nl(dotAll); options.set_log_errors(false); // inappropriate when embedding - std::unique_ptr re2(new WrappedRE2(re2::StringPiece(data, size), options, source, enabledCache, global, ignoreCase, multiline, dotAll, sticky, hasIndices)); + std::unique_ptr re2(new WrappedRE2(re2::StringPiece(data, size), options, source, global, ignoreCase, multiline, dotAll, sticky, hasIndices)); if (!re2->regexp.ok()) { return Nan::ThrowSyntaxError(re2->regexp.error().c_str()); diff --git a/lib/str-val.h b/lib/str-val.h index 8310273..330c7c9 100644 --- a/lib/str-val.h +++ b/lib/str-val.h @@ -1,4 +1,5 @@ #pragma once + #include #include #include diff --git a/lib/to_string.cc b/lib/to_string.cc index 6b899b9..b246ca5 100644 --- a/lib/to_string.cc +++ b/lib/to_string.cc @@ -20,10 +20,6 @@ NAN_METHOD(WrappedRE2::ToString) buffer += re2->source; buffer += "/"; - if (re2->enabledCache) - { - buffer += "\b"; - } if (re2->global) { buffer += "g"; diff --git a/lib/util.h b/lib/util.h index 9ba0c60..28d0c9c 100644 --- a/lib/util.h +++ b/lib/util.h @@ -1,5 +1,4 @@ -#ifndef UTIL_H_ -#define UTIL_H_ +#pragma once #include "./wrapped_re2.h" @@ -13,5 +12,3 @@ void consoleCall(const v8::Local &methodName, v8::Local t void printDeprecationWarning(const char *warning); v8::Local callToString(const v8::Local &object); - -#endif diff --git a/lib/wrapped_re2.h b/lib/wrapped_re2.h index 89fa68a..0a9a337 100644 --- a/lib/wrapped_re2.h +++ b/lib/wrapped_re2.h @@ -1,13 +1,9 @@ -#ifndef WRAPPED_RE2_H_ -#define WRAPPED_RE2_H_ +#pragma once +#include #include - #include -#include -#include - struct StrValBase; class WrappedRE2 : public Nan::ObjectWrap @@ -17,7 +13,6 @@ class WrappedRE2 : public Nan::ObjectWrap const re2::StringPiece &pattern, const re2::RE2::Options &options, const std::string &src, - const bool &c, const bool &g, const bool &i, const bool &m, @@ -25,7 +20,6 @@ class WrappedRE2 : public Nan::ObjectWrap const bool &y, const bool &d) : regexp(pattern, options), source(src), - enabledCache(c), global(g), ignoreCase(i), multiline(m), @@ -50,8 +44,6 @@ class WrappedRE2 : public Nan::ObjectWrap static NAN_GETTER(GetLastIndex); static NAN_SETTER(SetLastIndex); static NAN_GETTER(GetInternalSource); - static NAN_GETTER(GetEnabledCache); - static NAN_GETTER(GetIsCached); // RegExp methods static NAN_METHOD(Exec); @@ -91,7 +83,6 @@ class WrappedRE2 : public Nan::ObjectWrap re2::RE2 regexp; std::string source; - bool enabledCache; bool global; bool ignoreCase; bool multiline; @@ -104,16 +95,9 @@ class WrappedRE2 : public Nan::ObjectWrap private: Nan::Persistent lastString; // weak pointer + Nan::Persistent lastCache; // weak pointer StrValBase *lastStringValue; - typedef WrappedRE2 *PtrWrappedRE2; - - std::unordered_set callbackRegistry; - PtrWrappedRE2 *registerCallback(); - void unregisterCallback(PtrWrappedRE2 *re2); - - static void weakLastStringCallback(const Nan::WeakCallbackInfo &data); - void dropLastString(); void prepareLastString(const v8::Local &arg, bool ignoreLastIndex = false); }; @@ -125,12 +109,6 @@ struct PrepareLastString re2->prepareLastString(arg, ignoreLastIndex); } - ~PrepareLastString() - { - if (!re2->enabledCache || !(re2->global || re2->sticky)) - re2->dropLastString(); - } - WrappedRE2 *re2; }; @@ -211,5 +189,3 @@ inline size_t getUtf8CharSize(char ch) { return ((0xE5000000 >> ((ch >> 3) & 0x1E)) & 3) + 1; } - -#endif diff --git a/re2.js b/re2.js index 6786d42..3f32be9 100644 --- a/re2.js +++ b/re2.js @@ -24,7 +24,7 @@ if (typeof Symbol != 'undefined') { if (!this.global) { throw TypeError('String.prototype.matchAll called with a non-global RE2 argument'); } - const re = new RE2(this, this.flags + '\b'); + const re = new RE2(this); re.lastIndex = this.lastIndex; for (;;) { const result = re.exec(str);