Skip to content

Commit

Permalink
Made the cache of the last string a weak pointer. Refs #211.
Browse files Browse the repository at this point in the history
  • Loading branch information
uhop committed Jun 6, 2024
1 parent 52638bc commit 403368f
Show file tree
Hide file tree
Showing 8 changed files with 40 additions and 104 deletions.
28 changes: 0 additions & 28 deletions lib/accessors.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<WrappedRE2>(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<WrappedRE2>(info.This());
info.GetReturnValue().Set(!!re2->lastStringValue);
}

NAN_GETTER(WrappedRE2::GetFlags)
{
if (!WrappedRE2::HasInstance(info.This()))
Expand All @@ -63,10 +39,6 @@ NAN_GETTER(WrappedRE2::GetFlags)
auto re2 = Nan::ObjectWrap::Unwrap<WrappedRE2>(info.This());

std::string flags;
if (re2->enabledCache)
{
flags += "\b";
}
if (re2->hasIndices)
{
flags += "d";
Expand Down
67 changes: 33 additions & 34 deletions lib/addon.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ v8::Local<v8::Function> 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();

Expand All @@ -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<PtrWrappedRE2> &data)
static void weakLastCacheCallback(const Nan::WeakCallbackInfo<StrValBase> &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<v8::Value> &arg, bool ignoreLastIndex)
Expand All @@ -152,21 +135,37 @@ void WrappedRE2::prepareLastString(const v8::Local<v8::Value> &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<StrValString *>(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<v8::PersistentBase<v8::Value> &>(lastString).SetWeak();

Nan::Persistent<v8::Value> dummy(arg);
dummy.SetWeak(registerCallback(), weakLastStringCallback, Nan::WeakCallbackType::kParameter);
// create a holder object for the cache
v8::Local<v8::ObjectTemplate> objectTemplate = Nan::New<v8::ObjectTemplate>();
objectTemplate->SetInternalFieldCount(1);
v8::Local<v8::Object> 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<v8::Object> placeHolderForCache(object);
placeHolderForCache.SetWeak(lastStringValue, weakLastCacheCallback, Nan::WeakCallbackType::kParameter);

// keep a weak pointer to the cache
lastCache.Reset(object);
static_cast<v8::PersistentBase<v8::Object> &>(lastCache).SetWeak();
};
7 changes: 1 addition & 6 deletions lib/new.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -257,9 +256,6 @@ NAN_METHOD(WrappedRE2::New)
{
switch (data[i])
{
case '\b':
enabledCache = true;
break;
case 'g':
global = true;
break;
Expand Down Expand Up @@ -343,7 +339,6 @@ NAN_METHOD(WrappedRE2::New)

if (needFlags)
{
enabledCache = re2->enabledCache;
global = re2->global;
ignoreCase = re2->ignoreCase;
multiline = re2->multiline;
Expand Down Expand Up @@ -406,7 +401,7 @@ NAN_METHOD(WrappedRE2::New)
options.set_dot_nl(dotAll);
options.set_log_errors(false); // inappropriate when embedding

std::unique_ptr<WrappedRE2> re2(new WrappedRE2(re2::StringPiece(data, size), options, source, enabledCache, global, ignoreCase, multiline, dotAll, sticky, hasIndices));
std::unique_ptr<WrappedRE2> 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());
Expand Down
1 change: 1 addition & 0 deletions lib/str-val.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#pragma once

#include <vector>
#include <nan.h>
#include <re2/re2.h>
Expand Down
4 changes: 0 additions & 4 deletions lib/to_string.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,6 @@ NAN_METHOD(WrappedRE2::ToString)
buffer += re2->source;
buffer += "/";

if (re2->enabledCache)
{
buffer += "\b";
}
if (re2->global)
{
buffer += "g";
Expand Down
5 changes: 1 addition & 4 deletions lib/util.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#ifndef UTIL_H_
#define UTIL_H_
#pragma once

#include "./wrapped_re2.h"

Expand All @@ -13,5 +12,3 @@ void consoleCall(const v8::Local<v8::String> &methodName, v8::Local<v8::Value> t
void printDeprecationWarning(const char *warning);

v8::Local<v8::String> callToString(const v8::Local<v8::Object> &object);

#endif
30 changes: 3 additions & 27 deletions lib/wrapped_re2.h
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
#ifndef WRAPPED_RE2_H_
#define WRAPPED_RE2_H_
#pragma once

#include <string>
#include <nan.h>

#include <re2/re2.h>

#include <string>
#include <unordered_set>

struct StrValBase;

class WrappedRE2 : public Nan::ObjectWrap
Expand All @@ -17,15 +13,13 @@ 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,
const bool &s,
const bool &y,
const bool &d) : regexp(pattern, options),
source(src),
enabledCache(c),
global(g),
ignoreCase(i),
multiline(m),
Expand All @@ -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);
Expand Down Expand Up @@ -91,7 +83,6 @@ class WrappedRE2 : public Nan::ObjectWrap

re2::RE2 regexp;
std::string source;
bool enabledCache;
bool global;
bool ignoreCase;
bool multiline;
Expand All @@ -104,16 +95,9 @@ class WrappedRE2 : public Nan::ObjectWrap

private:
Nan::Persistent<v8::Value> lastString; // weak pointer
Nan::Persistent<v8::Object> lastCache; // weak pointer
StrValBase *lastStringValue;

typedef WrappedRE2 *PtrWrappedRE2;

std::unordered_set<PtrWrappedRE2 *> callbackRegistry;
PtrWrappedRE2 *registerCallback();
void unregisterCallback(PtrWrappedRE2 *re2);

static void weakLastStringCallback(const Nan::WeakCallbackInfo<PtrWrappedRE2> &data);

void dropLastString();
void prepareLastString(const v8::Local<v8::Value> &arg, bool ignoreLastIndex = false);
};
Expand All @@ -125,12 +109,6 @@ struct PrepareLastString
re2->prepareLastString(arg, ignoreLastIndex);
}

~PrepareLastString()
{
if (!re2->enabledCache || !(re2->global || re2->sticky))
re2->dropLastString();
}

WrappedRE2 *re2;
};

Expand Down Expand Up @@ -211,5 +189,3 @@ inline size_t getUtf8CharSize(char ch)
{
return ((0xE5000000 >> ((ch >> 3) & 0x1E)) & 3) + 1;
}

#endif
2 changes: 1 addition & 1 deletion re2.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 403368f

Please sign in to comment.