diff --git a/apple/MarkdownParser.h b/apple/MarkdownParser.h index 407e49b0..1d73622d 100644 --- a/apple/MarkdownParser.h +++ b/apple/MarkdownParser.h @@ -8,6 +8,26 @@ NS_ASSUME_NONNULL_BEGIN - (NSArray *)parse:(nonnull NSString *)text withParserId:(nonnull NSNumber *)parserId; -NS_ASSUME_NONNULL_END +// Returns the ranges for (text, parserId) if they are already in the cache, or +// nil if they aren't. This never runs the parser, so it is safe to call from +// the main thread while Yoga is measuring. +- (nullable NSArray *)cachedRangesForText:(nonnull NSString *)text + withParserId:(nonnull NSNumber *)parserId; + +// Parses in the background and puts the result in the cache, so the caller +// doesn't have to wait for the parser. Used by the main thread, which must +// never wait for it (see Sentry APP-EF1). +// +// Only the newest request matters: a new call replaces one that is still +// waiting. A parse that already started can't be stopped, but the newest text +// is parsed as soon as it finishes. +// +// `completion` runs on the background queue once the text is cached. It is +// skipped if a newer call replaced this one, since that call reports instead. +- (void)warmCacheAsyncForText:(nonnull NSString *)text + withParserId:(nonnull NSNumber *)parserId + completion:(nullable void (^)(void))completion; @end + +NS_ASSUME_NONNULL_END diff --git a/apple/MarkdownParser.mm b/apple/MarkdownParser.mm index fe469612..89bb209b 100644 --- a/apple/MarkdownParser.mm +++ b/apple/MarkdownParser.mm @@ -2,77 +2,257 @@ #import #import +// The main thread can only use ranges that are already cached (see +// RCTMarkdownUtils), and one entry was not enough: two texts taking turns kept +// overwriting each other, so the main thread never found what it needed. A few +// entries cover that case (typing then undoing, or an input echoing back older +// text), and the list is still small enough to scan one by one. +static const NSUInteger kMarkdownParserCacheCapacity = 4; + +@interface MarkdownParserCacheEntry : NSObject + +@property (nonatomic, readonly, nonnull) NSString *text; +@property (nonatomic, readonly, nonnull) NSNumber *parserId; +@property (nonatomic, readonly, nonnull) NSArray *markdownRanges; + +- (instancetype)initWithText:(nonnull NSString *)text + parserId:(nonnull NSNumber *)parserId + markdownRanges:(nonnull NSArray *)markdownRanges; + +- (BOOL)matchesText:(nonnull NSString *)text parserId:(nonnull NSNumber *)parserId; + +@end + +@implementation MarkdownParserCacheEntry + +- (instancetype)initWithText:(nonnull NSString *)text + parserId:(nonnull NSNumber *)parserId + markdownRanges:(nonnull NSArray *)markdownRanges +{ + if (self = [super init]) { + _text = [text copy]; + _parserId = parserId; + _markdownRanges = markdownRanges; + } + + return self; +} + +- (BOOL)matchesText:(nonnull NSString *)text parserId:(nonnull NSNumber *)parserId +{ + // Check the parser id first - comparing numbers is cheaper than strings. + return [_parserId isEqualToNumber:parserId] && [_text isEqualToString:text]; +} + +@end + +// Everything below is only read and written inside `@synchronized (self)`. @implementation MarkdownParser { - NSString *_prevText; - NSNumber *_prevParserId; - NSArray *_prevMarkdownRanges; + // Newest entry first, at index 0. + NSMutableArray *_cache; + + // The next text to parse in the background, if there is one. + NSString *_pendingText; + NSNumber *_pendingParserId; + void (^_pendingCompletion)(void); + BOOL _warmupScheduled; } -- (NSArray *)parse:(nonnull NSString *)text - withParserId:(nonnull NSNumber *)parserId +- (instancetype)init +{ + if (self = [super init]) { + _cache = [[NSMutableArray alloc] initWithCapacity:kMarkdownParserCacheCapacity]; + } + + return self; +} + +// One background queue for parsing off the main thread. A serial queue is +// enough: the worklet runtime only runs one parse at a time anyway, and this +// keeps us from doing the same work twice. ++ (dispatch_queue_t)cacheWarmupQueue +{ + static dispatch_queue_t queue; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ + dispatch_queue_attr_t attr = dispatch_queue_attr_make_with_qos_class( + DISPATCH_QUEUE_SERIAL, QOS_CLASS_USER_INITIATED, 0); + queue = dispatch_queue_create("com.expensify.livemarkdown.parser-cache-warmup", attr); + }); + return queue; +} + +- (nullable NSArray *)cachedRangesForText:(nonnull NSString *)text + withParserId:(nonnull NSNumber *)parserId { @synchronized (self) { - if ([text isEqualToString:_prevText] && [parserId isEqualToNumber:_prevParserId]) { - return _prevMarkdownRanges; + for (NSUInteger i = 0, n = _cache.count; i < n; i++) { + MarkdownParserCacheEntry *entry = _cache[i]; + if (![entry matchesText:text parserId:parserId]) { + continue; + } + if (i != 0) { + // Move it to the front, so the text we keep asking for is not the one + // we throw away next. + [_cache removeObjectAtIndex:i]; + [_cache insertObject:entry atIndex:0]; + } + return entry.markdownRanges; } + } + + return nil; +} - const auto &markdownRuntime = expensify::livemarkdown::getMarkdownRuntime(); - jsi::Runtime &rt = markdownRuntime->getJSIRuntime(); - - std::shared_ptr markdownWorklet; - try { - markdownWorklet = expensify::livemarkdown::getMarkdownWorklet([parserId intValue]); - } catch (const std::out_of_range &error) { - _prevText = [NSString stringWithString:text]; - _prevParserId = parserId; - _prevMarkdownRanges = @[]; - return _prevMarkdownRanges; +- (void)cacheMarkdownRanges:(nonnull NSArray *)markdownRanges + forText:(nonnull NSString *)text + withParserId:(nonnull NSNumber *)parserId +{ + MarkdownParserCacheEntry *entry = [[MarkdownParserCacheEntry alloc] initWithText:text + parserId:parserId + markdownRanges:markdownRanges]; + + @synchronized (self) { + for (NSUInteger i = 0, n = _cache.count; i < n; i++) { + if ([_cache[i] matchesText:text parserId:parserId]) { + [_cache removeObjectAtIndex:i]; + break; + } } + [_cache insertObject:entry atIndex:0]; + while (_cache.count > kMarkdownParserCacheCapacity) { + [_cache removeLastObject]; + } + } +} + +- (void)warmCacheAsyncForText:(nonnull NSString *)text + withParserId:(nonnull NSNumber *)parserId + completion:(nullable void (^)(void))completion +{ + @synchronized (self) { + // Keep only the newest request: replace anything that is waiting but + // hasn't started, so old text can never win over newer text. The replaced + // completion goes with it, since the new request will report instead. + _pendingText = [text copy]; + _pendingParserId = parserId; + _pendingCompletion = completion; - const auto &input = jsi::String::createFromUtf8(rt, [text UTF8String]); - - jsi::Value output; - try { - output = markdownRuntime->runGuarded(markdownWorklet, input); - } catch (const jsi::JSError &error) { - // Skip formatting, runGuarded will show the error in LogBox - _prevText = [NSString stringWithString:text]; - _prevParserId = parserId; - _prevMarkdownRanges = @[]; - return _prevMarkdownRanges; + if (_warmupScheduled) { + // A background loop is already running and will pick this up. + return; } + _warmupScheduled = YES; + } - NSMutableArray *markdownRanges = [[NSMutableArray alloc] init]; - try { - const auto &ranges = output.asObject(rt).asArray(rt); - for (size_t i = 0, n = ranges.size(rt); i < n; ++i) { - const auto &item = ranges.getValueAtIndex(rt, i).asObject(rt); - const auto &type = item.getProperty(rt, "type").asString(rt).utf8(rt); - const auto &start = static_cast(item.getProperty(rt, "start").asNumber()); - const auto &length = static_cast(item.getProperty(rt, "length").asNumber()); - const auto &depth = item.hasProperty(rt, "depth") ? static_cast(item.getProperty(rt, "depth").asNumber()) : 1; - - if (length == 0 || start + length > text.length) { - continue; - } - - NSRange range = NSMakeRange(start, length); - MarkdownRange *markdownRange = [[MarkdownRange alloc] initWithType:@(type.c_str()) range:range depth:depth]; - [markdownRanges addObject:markdownRange]; + __weak MarkdownParser *weakSelf = self; + dispatch_async([MarkdownParser cacheWarmupQueue], ^{ + [weakSelf drainPendingWarmups]; + }); +} + +- (void)drainPendingWarmups +{ + while (true) { + NSString *text; + NSNumber *parserId; + void (^completion)(void); + + @synchronized (self) { + if (_pendingText == nil) { + _warmupScheduled = NO; + return; } - } catch (const jsi::JSError &error) { - RCTLogWarn(@"[react-native-live-markdown] Incorrect schema of worklet parser output: %s", error.getMessage().c_str()); - _prevText = [NSString stringWithString:text]; - _prevParserId = parserId; - _prevMarkdownRanges = @[]; - return _prevMarkdownRanges; + text = _pendingText; + parserId = _pendingParserId; + completion = _pendingCompletion; + _pendingText = nil; + _pendingParserId = nil; + _pendingCompletion = nil; + } + + [self parse:text withParserId:parserId]; + + BOOL superseded; + @synchronized (self) { + superseded = _pendingText != nil; + } + if (completion != nil && !superseded) { + completion(); } + } +} + +- (NSArray *)parse:(nonnull NSString *)text + withParserId:(nonnull NSNumber *)parserId +{ + NSArray *cached = [self cachedRangesForText:text withParserId:parserId]; + if (cached != nil) { + return cached; + } + + // Parse WITHOUT holding an Objective-C lock. Running the parser waits for the + // worklet runtime, and holding a lock while waiting is what froze the app + // (Sentry APP-EF1): a background thread held this lock and waited on the + // runtime, so the main thread was stuck waiting for the lock while measuring, + // until iOS killed the app. + // + // Two threads may end up parsing the same text at the same time. That is + // fine: they run one after the other and produce the same result. + NSArray *markdownRanges = [self parseUncached:text withParserId:parserId]; + + [self cacheMarkdownRanges:markdownRanges forText:text withParserId:parserId]; - _prevText = [NSString stringWithString:text]; - _prevParserId = parserId; - _prevMarkdownRanges = markdownRanges; - return _prevMarkdownRanges; + return markdownRanges; +} + +- (NSArray *)parseUncached:(nonnull NSString *)text + withParserId:(nonnull NSNumber *)parserId +{ + const auto &markdownRuntime = expensify::livemarkdown::getMarkdownRuntime(); + jsi::Runtime &rt = markdownRuntime->getJSIRuntime(); + + std::shared_ptr markdownWorklet; + try { + markdownWorklet = expensify::livemarkdown::getMarkdownWorklet([parserId intValue]); + } catch (const std::out_of_range &error) { + return @[]; } + + const auto &input = jsi::String::createFromUtf8(rt, [text UTF8String]); + + jsi::Value output; + try { + output = markdownRuntime->runGuarded(markdownWorklet, input); + } catch (const jsi::JSError &error) { + // Skip formatting, runGuarded will show the error in LogBox + return @[]; + } + + NSMutableArray *markdownRanges = [[NSMutableArray alloc] init]; + try { + const auto &ranges = output.asObject(rt).asArray(rt); + for (size_t i = 0, n = ranges.size(rt); i < n; ++i) { + const auto &item = ranges.getValueAtIndex(rt, i).asObject(rt); + const auto &type = item.getProperty(rt, "type").asString(rt).utf8(rt); + const auto &start = static_cast(item.getProperty(rt, "start").asNumber()); + const auto &length = static_cast(item.getProperty(rt, "length").asNumber()); + const auto &depth = item.hasProperty(rt, "depth") ? static_cast(item.getProperty(rt, "depth").asNumber()) : 1; + + if (length == 0 || start + length > text.length) { + continue; + } + + NSRange range = NSMakeRange(start, length); + MarkdownRange *markdownRange = [[MarkdownRange alloc] initWithType:@(type.c_str()) range:range depth:depth]; + [markdownRanges addObject:markdownRange]; + } + } catch (const jsi::JSError &error) { + RCTLogWarn(@"[react-native-live-markdown] Incorrect schema of worklet parser output: %s", error.getMessage().c_str()); + return @[]; + } + + return markdownRanges; } @end diff --git a/apple/MarkdownTextInputDecoratorShadowNode.h b/apple/MarkdownTextInputDecoratorShadowNode.h index b821ab2d..b13ae0bc 100644 --- a/apple/MarkdownTextInputDecoratorShadowNode.h +++ b/apple/MarkdownTextInputDecoratorShadowNode.h @@ -8,6 +8,9 @@ #include #include +#include +#include + namespace facebook { namespace react { @@ -49,11 +52,15 @@ class JSI_EXPORT MarkdownTextInputDecoratorShadowNode final static YogaLayoutableShadowNode & shadowNodeFromContext(YGNodeConstRef yogaNode); - // Persisted RCTMarkdownUtils instance shared across shadow node clones so - // that MarkdownParser's one-entry memo cache (keyed on text + parserId) - // survives repeated Yoga measure callbacks instead of being discarded on - // every call to applyMarkdownFormattingToTextInputState. + // Shared between shadow node clones, so the parser cache survives all the + // cloning that happens during layout instead of being thrown away on every + // call to applyMarkdownFormattingToTextInputState. mutable std::shared_ptr markdownUtils_; + + // Set from any thread when a background parse finishes. + // overwriteMeasureCallbackConnector then marks the child's Yoga node dirty so + // it gets measured again with markdown. Passed along with markdownUtils_. + mutable std::shared_ptr needsRemeasure_; }; } // namespace react diff --git a/apple/MarkdownTextInputDecoratorShadowNode.mm b/apple/MarkdownTextInputDecoratorShadowNode.mm index 57c88dd0..134eff8c 100644 --- a/apple/MarkdownTextInputDecoratorShadowNode.mm +++ b/apple/MarkdownTextInputDecoratorShadowNode.mm @@ -3,9 +3,13 @@ #include #include #include +#include #include #include +#include +#include + #include "RCTMarkdownStyle.h" #include "RCTMarkdownUtils.h" @@ -32,12 +36,13 @@ ShadowNode const &sourceShadowNode, ShadowNodeFragment const &fragment) : ConcreteViewShadowNode(sourceShadowNode, fragment) { - // Carry the persisted RCTMarkdownUtils over from the source node so the - // MarkdownParser memo cache survives the frequent cloning that happens - // during layout and re-render cycles. + // Copy the utils (and its parser cache) and the re-measure flag from the node + // we are cloning. Both have to be set before makeChildNodeMutable() below, + // which is what reads the flag. const auto &source = static_cast(sourceShadowNode); markdownUtils_ = source.markdownUtils_; + needsRemeasure_ = source.needsRemeasure_; initialize(); makeChildNodeMutable(); @@ -97,6 +102,20 @@ // on the decorator const auto &yogaNode = &nodeWithAccessibleYogaNode->yogaNode_; YGNodeSetMeasureFunc(yogaNode, yogaNodeMeasureCallbackConnector); + + // If a background parse finished since the last layout, the size Yoga saved + // for the child was measured from plain text, and nothing marks this part of + // the tree dirty on its own (Yoga does not measure the decorator), so the + // wrong height would stay until something else changed the layout. + // + // Mark both nodes dirty by hand instead of using YGNodeMarkDirty(): the child + // is usually dirty already from completeClone(), which stops the flag from + // travelling up and would leave the decorator clean. Parents pick it up on + // their own when updateYogaChildren() runs. + if (needsRemeasure_ != nullptr && needsRemeasure_->exchange(false)) { + yogaNode->setDirty(true); + yogaNode_.setDirty(true); + } } void MarkdownTextInputDecoratorShadowNode::appendChild( @@ -189,14 +208,35 @@ const auto defaultNSTextAttributes = RCTNSTextAttributesFromTextAttributes(defaultTextAttributes); - // Lazily create and persist the RCTMarkdownUtils instance so the MarkdownParser - // one-entry memo cache (keyed on text + parserId) survives repeated Yoga measure - // callbacks. Previously a fresh utils/parser was allocated on every call, - // discarding the cache and forcing a full JSI re-parse each time. + // Create the utils once and keep it, so the parser cache survives repeated + // measure calls. It used to be created fresh every time, which threw the + // cache away and forced a full re-parse. if (!markdownUtils_) { RCTMarkdownUtils *freshUtils = [[RCTMarkdownUtils alloc] init]; markdownUtils_ = std::shared_ptr( (__bridge_retained void *)freshUtils, [](void *p) { CFRelease(p); }); + needsRemeasure_ = std::make_shared(false); + + // When the main thread does not find the ranges in the cache, it measures + // plain text and parses in the background instead (see RCTMarkdownUtils). + // That size is wrong for any style that changes how big the text is, so + // once the ranges arrive we force another measure rather than hope + // something else does. Both steps are needed: the state update schedules a + // new commit, and the flag makes Yoga measure again instead of reusing the + // old size. updateState() can be called from any thread - it only queues + // work on the family. + const auto state = + std::static_pointer_cast>(getState()); + if (state != nullptr) { + const auto needsRemeasure = needsRemeasure_; + freshUtils.onAsyncFormattingReady = ^{ + needsRemeasure->store(true); + // This update changes nothing; it is here only to schedule a new + // commit. If the family is already gone, updateState() handles that. + state->updateState(MarkdownTextInputDecoratorState{}); + }; + } } RCTMarkdownUtils *utils = (__bridge RCTMarkdownUtils *)markdownUtils_.get(); diff --git a/apple/RCTMarkdownUtils.h b/apple/RCTMarkdownUtils.h index 9b290783..e9978442 100644 --- a/apple/RCTMarkdownUtils.h +++ b/apple/RCTMarkdownUtils.h @@ -8,13 +8,23 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic) RCTMarkdownStyle *markdownStyle; @property (nonatomic) NSNumber *parserId; +// Called on a background queue once a background parse has finished and the +// ranges are cached. The owner should mark the layout as out of date so the +// text is measured again, this time with markdown. Otherwise the size measured +// from plain text stays until something else triggers a new layout. +@property (atomic, copy, nullable) void (^onAsyncFormattingReady)(void); + - (void)applyMarkdownFormatting:(nonnull NSMutableAttributedString *)attributedString withDefaultTextAttributes:(nonnull NSDictionary *)defaultTextAttributes; -// Atomically sets the style/parser and applies formatting under a single lock. -// Use this from the shadow node measure path, where one RCTMarkdownUtils -// instance is shared across shadow node clones and may be accessed from -// concurrent Fabric commits/layout passes. +// Sets the style/parser and formats using the values passed in. Use this from +// the shadow node measure path, where one instance is shared between clones and +// several Fabric threads can call it at the same time. +// +// On the main thread this only uses ranges that are already cached and never +// runs the parser. If they are not there, the text is left unformatted for this +// pass and parsed in the background, then `onAsyncFormattingReady` is called so +// the node can be measured again (see Sentry APP-EF1). - (void)applyMarkdownFormatting:(nonnull NSMutableAttributedString *)attributedString withDefaultTextAttributes:(nonnull NSDictionary *)defaultTextAttributes markdownStyle:(nonnull RCTMarkdownStyle *)markdownStyle diff --git a/apple/RCTMarkdownUtils.mm b/apple/RCTMarkdownUtils.mm index b2919d55..aa034dfe 100644 --- a/apple/RCTMarkdownUtils.mm +++ b/apple/RCTMarkdownUtils.mm @@ -39,17 +39,54 @@ - (void)applyMarkdownFormatting:(nonnull NSMutableAttributedString *)attributedS markdownStyle:(nonnull RCTMarkdownStyle *)markdownStyle parserId:(nonnull NSNumber *)parserId { - // Keep the style/parserId assignment and the parse+format together under a - // single lock. The shadow node shares one instance across clones, and Fabric - // runs commits/layout optimistically on multiple threads, so without this the - // setters could interleave with another thread's parse/format and apply the - // wrong parserId/style for a frame. `@synchronized` is recursive, so nesting - // with `MarkdownParser`'s own `@synchronized(self)` in `parse:` is safe. + // The lock only covers the two shared fields. Holding it while parsing and + // formatting froze the app (Sentry APP-EF1): parsing waits for the worklet + // runtime, so a background thread could hold this lock while waiting, leaving + // the main thread stuck on the lock while measuring. The formatting below + // uses the arguments instead of the fields, so every call still gets a + // matching style and parserId. @synchronized (self) { _markdownStyle = markdownStyle; _parserId = parserId; - [self applyMarkdownFormatting:attributedString withDefaultTextAttributes:defaultTextAttributes]; } + + NSString *text = attributedString.string; + NSArray *markdownRanges = [_markdownParser cachedRangesForText:text withParserId:parserId]; + + if (markdownRanges == nil) { + if ([NSThread isMainThread]) { + // Never run the parser from the main thread while measuring. If the + // worklet runtime is busy, the wait can pass the ~2s limit and iOS kills + // the app (Sentry APP-EF1). Parse in the background instead and measure + // plain text for now; `onAsyncFormattingReady` asks for a new measure + // once the ranges are ready, so the wrong sizes (h1 font, code font, + // blockquote indent, emoji) do not stay. This rarely happens - text + // changes are usually parsed on a background thread first, so the main + // thread finds the ranges in the cache. + __weak RCTMarkdownUtils *weakSelf = self; + [_markdownParser warmCacheAsyncForText:text + withParserId:parserId + completion:^{ + // This only runs for the newest text, so it can't loop forever: the new + // measure finds the ranges in the cache (or the text changed again, and + // then a new measure was needed anyway). + void (^handler)(void) = weakSelf.onAsyncFormattingReady; + if (handler != nil) { + handler(); + } + }]; + return; + } + // Background threads can parse right here: the ~2s limit only applies to + // the main thread, and parsing no longer holds a lock the main thread + // waits on. + markdownRanges = [_markdownParser parse:text withParserId:parserId]; + } + + [_markdownFormatter formatAttributedString:attributedString + withDefaultTextAttributes:defaultTextAttributes + withMarkdownRanges:markdownRanges + withMarkdownStyle:markdownStyle]; } @end