From 53c1eee641353b1f5ca2fde8a0737159da0f5a3b Mon Sep 17 00:00:00 2001 From: Pritesh Nandgaonkar Date: Thu, 8 Aug 2019 09:13:32 -0700 Subject: [PATCH] Change the implementation of fresco plugin on android Summary: This updates the implementation of the export of the fresco plugin on the android side. Initially we used to send all the images info in one call, but its not scalable, as the images increase, the payload size of the rsocket can increase which will lead rsocket to drop messages. This diff updates the implementation of android side to match the one on the iOS side. Reviewed By: passy Differential Revision: D16627823 fbshipit-source-id: 563bf5fb20595c198b6447bb4e41f04af6e46644 --- .../plugins/fresco/FrescoFlipperPlugin.java | 70 ++------- src/plugins/fresco/index.js | 138 ++++++------------ 2 files changed, 64 insertions(+), 144 deletions(-) diff --git a/android/src/main/java/com/facebook/flipper/plugins/fresco/FrescoFlipperPlugin.java b/android/src/main/java/com/facebook/flipper/plugins/fresco/FrescoFlipperPlugin.java index a8646a08a..8a10332db 100644 --- a/android/src/main/java/com/facebook/flipper/plugins/fresco/FrescoFlipperPlugin.java +++ b/android/src/main/java/com/facebook/flipper/plugins/fresco/FrescoFlipperPlugin.java @@ -127,54 +127,13 @@ public class FrescoFlipperPlugin extends BufferingFlipperPlugin public void onConnect(FlipperConnection connection) { super.onConnect(connection); connection.receive( - "getAllImageData", + "getAllImageEventsInfo", new FlipperReceiver() { @Override - public void onReceive(FlipperObject params, FlipperResponder responder) { + public void onReceive(FlipperObject params, FlipperResponder responder) throws Exception { if (!ensureFrescoInitialized(responder)) { return; } - final ImagePipelineFactory imagePipelineFactory = Fresco.getImagePipelineFactory(); - final CountingMemoryCacheInspector.DumpInfo memoryCache = - new CountingMemoryCacheInspector<>( - imagePipelineFactory.getBitmapCountingMemoryCache()) - .dumpCacheContent(); - - final FlipperArray memoryCacheSharedEntries = - buildImageIdList(memoryCache.sharedEntries); - final FlipperArray memoryCacheLRUEntries = buildImageIdList(memoryCache.lruEntries); - final FlipperArray.Builder imageIDListBuilder = new FlipperArray.Builder(); - for (int i = 0; i < memoryCacheSharedEntries.length(); ++i) { - final String imageID = memoryCacheSharedEntries.getString(i); - imageIDListBuilder.put(imageID); - } - for (int i = 0; i < memoryCacheLRUEntries.length(); ++i) { - final String imageID = memoryCacheLRUEntries.getString(i); - imageIDListBuilder.put(imageID); - } - final FlipperArray imageIDs = imageIDListBuilder.build(); - final FlipperObject levels = - getImageList(memoryCache, memoryCacheSharedEntries, memoryCacheLRUEntries); - final FlipperArray.Builder imageDataListBuilder = new FlipperArray.Builder(); - - for (int i = 0; i < imageIDs.length(); ++i) { - final String imageID = imageIDs.getString(i); - final CacheKey cacheKey = mFlipperImageTracker.getCacheKey(imageID); - if (cacheKey == null) { - continue; - } - final CloseableReference ref = - imagePipelineFactory.getBitmapCountingMemoryCache().get(cacheKey); - if (ref == null) { - continue; - } - final CloseableBitmap bitmap = (CloseableBitmap) ref.get(); - final String encodedBitmap = - bitmapToBase64Preview(bitmap.getUnderlyingBitmap(), mPlatformBitmapFactory); - imageDataListBuilder.put( - getImageData( - imageID, encodedBitmap, bitmap, mFlipperImageTracker.getUriString(cacheKey))); - } FlipperArray.Builder arrayBuilder = new FlipperArray.Builder(); for (FlipperObject obj : mEvents) { @@ -183,14 +142,11 @@ public class FrescoFlipperPlugin extends BufferingFlipperPlugin mEvents.clear(); FlipperObject object = - new FlipperObject.Builder() - .put("levels", levels) - .put("imageDataList", imageDataListBuilder.build()) - .put("events", arrayBuilder.build()) - .build(); + new FlipperObject.Builder().put("events", arrayBuilder.build()).build(); responder.success(object); } }); + connection.receive( "listImages", new FlipperReceiver() { @@ -241,14 +197,20 @@ public class FrescoFlipperPlugin extends BufferingFlipperPlugin mPerfLogger.cancelMarker("Sonar.Fresco.getImage"); return; } - final CloseableBitmap bitmap = (CloseableBitmap) ref.get(); - String encodedBitmap = - bitmapToBase64Preview(bitmap.getUnderlyingBitmap(), mPlatformBitmapFactory); - responder.success( - getImageData( - imageId, encodedBitmap, bitmap, mFlipperImageTracker.getUriString(cacheKey))); + if (ref.get() instanceof CloseableBitmap) { + final CloseableBitmap bitmap = (CloseableBitmap) ref.get(); + String encodedBitmap = + bitmapToBase64Preview(bitmap.getUnderlyingBitmap(), mPlatformBitmapFactory); + responder.success( + getImageData( + imageId, encodedBitmap, bitmap, mFlipperImageTracker.getUriString(cacheKey))); + } else { + // TODO: T48376327, it might happened that ref.get() may not be casted to + // CloseableBitmap, this issue is tracked in the before mentioned task + responder.success(); + } mPerfLogger.endMarker("Sonar.Fresco.getImage"); } }); diff --git a/src/plugins/fresco/index.js b/src/plugins/fresco/index.js index 1e449c093..a0cea3a83 100644 --- a/src/plugins/fresco/index.js +++ b/src/plugins/fresco/index.js @@ -109,105 +109,63 @@ export default class FlipperImagesPlugin extends FlipperPlugin< return defaultPromise; } const selectedDevice = store.getState().connections.selectedDevice; - if (selectedDevice && selectedDevice.os === 'iOS') { - return Promise.all([ - callClient('listImages'), - callClient('getAllImageEventsInfo'), - ]).then(async ([responseImages, responseEvents]) => { - const levels: ImagesList = responseImages.levels; - const events: Array = responseEvents.events; - let pluginData: PersistedState = { - ...persistedState, - images: persistedState - ? [...persistedState.images, ...levels] - : levels, - closeableReferenceLeaks: - (persistedState && persistedState.closeableReferenceLeaks) || [], - }; + return Promise.all([ + callClient('listImages'), + callClient('getAllImageEventsInfo'), + ]).then(async ([responseImages, responseEvents]) => { + const levels: ImagesList = responseImages.levels; + const events: Array = responseEvents.events; + let pluginData: PersistedState = { + ...persistedState, + images: persistedState ? [...persistedState.images, ...levels] : levels, + closeableReferenceLeaks: + (persistedState && persistedState.closeableReferenceLeaks) || [], + }; - events.forEach((event: ImageEventWithId, index) => { - if (!event) { - return; + events.forEach((event: ImageEventWithId, index) => { + if (!event) { + return; + } + const {attribution} = event; + if ( + attribution && + attribution instanceof Array && + attribution.length > 0 + ) { + const surface = attribution[0] ? attribution[0].trim() : undefined; + if (surface && surface.length > 0) { + pluginData.surfaceList.add(surface); } - const {attribution} = event; - if ( - attribution && - attribution instanceof Array && - attribution.length > 0 - ) { - const surface = attribution[0].trim(); - if (surface.length > 0) { - pluginData.surfaceList.add(surface); - } - } - pluginData = { - ...pluginData, - events: [{eventId: index, ...event}, ...pluginData.events], - }; + } + pluginData = { + ...pluginData, + events: [{eventId: index, ...event}, ...pluginData.events], + }; + }); + const idSet: Set = levels.reduce((acc, level: CacheInfo) => { + level.imageIds.forEach(id => { + acc.add(id); }); - const idSet: Set = levels.reduce((acc, level: CacheInfo) => { - level.imageIds.forEach(id => { - acc.add(id); - }); - return acc; - }, new Set()); - const imageDataList: Array = []; - for (const id: string of idSet) { + return acc; + }, new Set()); + const imageDataList: Array = []; + for (const id: string of idSet) { + try { const imageData: ImageData = await callClient('getImage', { imageId: id, }); imageDataList.push(imageData); + } catch (e) { + console.error(e); } - imageDataList.forEach((data: ImageData) => { - const imagesMap = {...pluginData.imagesMap}; - imagesMap[data.imageId] = data; - pluginData.imagesMap = imagesMap; - }); - return pluginData; + } + imageDataList.forEach((data: ImageData) => { + const imagesMap = {...pluginData.imagesMap}; + imagesMap[data.imageId] = data; + pluginData.imagesMap = imagesMap; }); - } else { - return callClient('getAllImageData').then((data: ImagesMetaData) => { - if (!data) { - return; - } - const {levels, events, imageDataList} = data; - let pluginData: PersistedState = { - ...persistedState, - images: persistedState - ? [...persistedState.images, ...levels.levels] - : levels.levels, - closeableReferenceLeaks: - (persistedState && persistedState.closeableReferenceLeaks) || [], - }; - - events.forEach((event: ImageEventWithId, index) => { - if (!event) { - return; - } - const {attribution} = event; - if ( - attribution && - attribution instanceof Array && - attribution.length > 0 - ) { - const surface = attribution[0] ? attribution[0].trim() : undefined; - if (surface && surface.length > 0) { - pluginData.surfaceList.add(surface); - } - } - pluginData = { - ...pluginData, - events: [{eventId: index, ...event}, ...pluginData.events], - }; - }); - - imageDataList.forEach((imageData: ImageData) => { - const {imageId} = imageData; - pluginData.imagesMap[imageId] = imageData; - }); - return pluginData; - }); - } + return pluginData; + }); }; static persistedStateReducer = (