diff --git a/apple/MarkdownParser.h b/apple/MarkdownParser.h index 407e49b0..4cc802a4 100644 --- a/apple/MarkdownParser.h +++ b/apple/MarkdownParser.h @@ -8,6 +8,30 @@ NS_ASSUME_NONNULL_BEGIN - (NSArray *)parse:(nonnull NSString *)text withParserId:(nonnull NSNumber *)parserId; -NS_ASSUME_NONNULL_END +// Returns the memoized ranges for (text, parserId) if they are still in the +// cache, otherwise nil. Never enters the worklet runtime and only takes a brief +// internal lock, so it is safe to call from the Yoga measure path on the main +// thread. +- (nullable NSArray *)cachedRangesForText:(nonnull NSString *)text + withParserId:(nonnull NSNumber *)parserId; + +// Requests a parse on a background queue to populate the cache without blocking +// the calling thread. Used when the main thread needs ranges during layout but +// must not wait on the worklet runtime (see Sentry APP-EF1). +// +// Requests are coalesced latest-wins: the most recent (text, parserId) always +// wins, and any earlier request that has not started executing yet is dropped. +// A parse that is already inside the worklet runtime cannot be cancelled, but +// the newest request is picked up as soon as it returns, so the newest text is +// always parsed. +// +// `completion` runs on the warm-up queue once `text` has been parsed and +// cached. It is skipped when the request was superseded by a newer one, since +// the newer request invokes its own completion. +- (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..c821ef55 100644 --- a/apple/MarkdownParser.mm +++ b/apple/MarkdownParser.mm @@ -2,77 +2,263 @@ #import #import +// Number of (text, parserId) results kept in the cache. +// +// The main-thread measure path can only format from this cache, because it must +// never enter the worklet runtime (see RCTMarkdownUtils). With a single entry +// that fast path was unreliable: two alternating text values thrashed the entry +// and every main-thread measure missed, falling back to measuring unformatted +// text. A handful of entries absorbs alternating values (e.g. typing then +// undoing, or a controlled input echoing back an older value) and is still +// cheap to scan linearly. +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 +{ + // Compare the parser id first, it is much cheaper than a string comparison. + return [_parserId isEqualToNumber:parserId] && [_text isEqualToString:text]; +} + +@end + @implementation MarkdownParser { - NSString *_prevText; - NSNumber *_prevParserId; - NSArray *_prevMarkdownRanges; + // Most-recently-used first; index 0 is the newest entry. Guarded by + // `@synchronized (self)`. + NSMutableArray *_cache; + + // Latest-wins coalescing state for background warm-ups. Guarded by + // `@synchronized (self)`. + 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; +} + +// Shared serial queue used to warm the cache off the main thread. A single queue +// is enough: parses are serialized by the markdown worklet runtime's own mutex +// anyway, and keeping it serial bounds duplicate work. ++ (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) { + // Refresh recency so the entry the measure path keeps asking for is not + // the one that gets evicted. + [_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) { + // Latest-wins: overwrite whatever was queued but has not started yet, so a + // stale request can never win over newer text. The completion of the + // superseded request is dropped together with it; its result would be stale + // and the newer request reports for itself. + _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 drain loop is already running (or queued) 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; + } + + // Run the worklet parse WITHOUT holding any Objective-C lock. Entering the + // shared worklet runtime acquires its recursive_mutex, and holding an ObjC + // lock across that boundary is what created the APP-EF1 lock-order + // inversion: the main thread got stuck in objc_sync_enter during Yoga + // measure while a background Fabric layout thread held the lock and waited + // on the runtime mutex, until the iOS watchdog killed the app. Concurrent + // cache misses may parse the same text twice; the runtime serializes them, + // the results are identical, and last-writer-wins on the cache is safe. + 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..81c03e71 100644 --- a/apple/MarkdownTextInputDecoratorShadowNode.h +++ b/apple/MarkdownTextInputDecoratorShadowNode.h @@ -8,6 +8,9 @@ #include #include +#include +#include + namespace facebook { namespace react { @@ -50,10 +53,17 @@ class JSI_EXPORT MarkdownTextInputDecoratorShadowNode final 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. + // that MarkdownParser's cache (keyed on text + parserId) survives repeated + // Yoga measure callbacks instead of being discarded on every call to + // applyMarkdownFormattingToTextInputState. mutable std::shared_ptr markdownUtils_; + + // Set from an arbitrary thread when an async markdown parse (scheduled by a + // main-thread measure that could not enter the worklet runtime) has finished. + // Shared across clones alongside markdownUtils_ and consumed by + // overwriteMeasureCallbackConnector, which dirties the child's Yoga node so + // the measure function runs again with markdown applied. + mutable std::shared_ptr needsRemeasure_; }; } // namespace react diff --git a/apple/MarkdownTextInputDecoratorShadowNode.mm b/apple/MarkdownTextInputDecoratorShadowNode.mm index 57c88dd0..7f10be75 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" @@ -33,11 +37,14 @@ 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. + // MarkdownParser cache survives the frequent cloning that happens during + // layout and re-render cycles. The re-measure flag has to travel with it, + // and both must be in place before makeChildNodeMutable() below, which is + // what consumes the flag. const auto &source = static_cast(sourceShadowNode); markdownUtils_ = source.markdownUtils_; + needsRemeasure_ = source.needsRemeasure_; initialize(); makeChildNodeMutable(); @@ -97,6 +104,26 @@ // on the decorator const auto &yogaNode = &nodeWithAccessibleYogaNode->yogaNode_; YGNodeSetMeasureFunc(yogaNode, yogaNodeMeasureCallbackConnector); + + // An async markdown parse may have completed since the last layout, meaning + // the measurement Yoga has cached for the child was taken from unformatted + // text (see the onAsyncFormattingReady handler in + // applyMarkdownFormattingToTextInputState). The state update that brought us + // here does not dirty this subtree on its own - the decorator is not a + // measurable Yoga node - so without this Yoga would reuse that stale + // measurement and the wrong height would stick until something else + // invalidated layout. + // + // Dirty both nodes explicitly instead of relying on + // YGNodeMarkDirty()'s upward propagation: the child is usually already dirty + // from completeClone(), in which case propagation short-circuits and the + // decorator would stay clean. Ancestors then pick the dirty flag up on their + // own, because they are cloned with new children and updateYogaChildren() + // propagates child dirtiness upwards. + if (needsRemeasure_ != nullptr && needsRemeasure_->exchange(false)) { + yogaNode->setDirty(true); + yogaNode_.setDirty(true); + } } void MarkdownTextInputDecoratorShadowNode::appendChild( @@ -190,13 +217,40 @@ 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. + // 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. 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 measure runs on the main thread it must not enter the worklet + // runtime, so on a cache miss it measures unformatted text and asks the + // parser to fill the cache in the background. That measurement is wrong for + // any style that changes text metrics, so once the ranges are ready we have + // to force another measure pass rather than hope something else dirties + // layout: + // 1. raise the flag, which makes the next clone dirty the child's Yoga + // node (see overwriteMeasureCallbackConnector), and + // 2. dispatch a state update on this family to produce that commit. + // Both are needed: the state update schedules the commit, the flag makes + // Yoga actually re-run the measure function instead of reusing its cache. + // updateState() is safe to call from any thread - it only enqueues work on + // the family's event dispatcher. + const auto state = + std::static_pointer_cast>(getState()); + if (state != nullptr) { + const auto needsRemeasure = needsRemeasure_; + freshUtils.onAsyncFormattingReady = ^{ + needsRemeasure->store(true); + // Cheap no-op state update; it exists only to schedule a commit. If the + // family is already gone updateState() bails out on its own. + state->updateState(MarkdownTextInputDecoratorState{}); + }; + } } RCTMarkdownUtils *utils = (__bridge RCTMarkdownUtils *)markdownUtils_.get(); diff --git a/apple/RCTMarkdownUtils.h b/apple/RCTMarkdownUtils.h index 9b290783..498e284f 100644 --- a/apple/RCTMarkdownUtils.h +++ b/apple/RCTMarkdownUtils.h @@ -8,13 +8,27 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic) RCTMarkdownStyle *markdownStyle; @property (nonatomic) NSNumber *parserId; +// Invoked from a background queue after an async parse scheduled by a +// main-thread measure pass has landed in the parser cache. The owner is expected +// to invalidate layout for the affected node so the text gets measured again, +// this time with markdown applied. Without it, the unformatted measurement taken +// during the cache miss could stick until something else dirtied layout. +// Thread-safe to set and read (atomic). +@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. +// Sets the style/parser and applies formatting using the given values. // 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. +// NOTE: on the main thread this formats only from the parser's cache and never +// enters the worklet runtime; on a cache miss it schedules an async parse and +// leaves the string unformatted for that measure pass, then calls +// `onAsyncFormattingReady` once the ranges are available so the node can be +// measured again. This keeps the main thread from blocking on runtime-bound +// locks during Yoga measure (Sentry APP-EF1 watchdog kills). - (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..ca26e7d5 100644 --- a/apple/RCTMarkdownUtils.mm +++ b/apple/RCTMarkdownUtils.mm @@ -39,17 +39,59 @@ - (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. + // Only protect the shared ivar updates with the lock. Holding + // `@synchronized(self)` across parse+format caused a lock-order inversion: + // `MarkdownParser parse:` synchronously enters the shared worklet runtime + // (recursive_mutex), so a background Fabric layout thread could hold this + // lock while blocked on the runtime mutex, leaving the main thread stuck in + // objc_sync_enter during Yoga measure until the watchdog killed the app + // (Sentry APP-EF1). Per-call consistency of style/parserId is preserved by + // formatting with the local parameters instead of re-reading the ivars. @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 enter the worklet runtime from the main thread during Yoga + // measure. If the runtime is busy (e.g. a background Fabric layout is + // parsing, or a worklet is blocked on another runtime), the wait can + // exceed the ~2s watchdog limit and iOS kills the app (Sentry APP-EF1). + // Parse asynchronously and measure with unformatted text for this pass; + // `onAsyncFormattingReady` then triggers a re-measure once the ranges are + // cached, so the temporarily wrong metrics (h1 font size, code/pre font, + // blockquote indent, emoji size) cannot persist. + // In practice the main thread usually hits the cache here: text changes + // are committed (and parsed) on background threads first. + __weak RCTMarkdownUtils *weakSelf = self; + [_markdownParser warmCacheAsyncForText:text + withParserId:parserId + completion:^{ + // Runs on the parser's warm-up queue, only for the newest requested + // text. Cannot spin: the re-measure it asks for hits the cache and stops + // scheduling warm-ups (or the text changed again, in which case the new + // text needs a re-measure anyway). + void (^handler)(void) = weakSelf.onAsyncFormattingReady; + if (handler != nil) { + handler(); + } + }]; + return; + } + // Background Fabric layout threads may parse synchronously: the watchdog + // only monitors the main thread, and parse no longer holds any lock the + // main-thread measure path can block on. + markdownRanges = [_markdownParser parse:text withParserId:parserId]; + } + + [_markdownFormatter formatAttributedString:attributedString + withDefaultTextAttributes:defaultTextAttributes + withMarkdownRanges:markdownRanges + withMarkdownStyle:markdownStyle]; } @end