Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 606ff191 authored by Jayant Chowdhary's avatar Jayant Chowdhary
Browse files

camera2: Reduce metadata lock contention 



Before this change, the native methods for metadata
access were static and synchronized, which meant that
all calls would contend for the same class lock.

This isn't actually necessary since the only true global
being accessed in native metadata is the VendorTagCache,
that is already protected by an internal lock in the native
code.

We do however, need to have the java methods which involve
reading and writing to metadata be synchronized since even though
CaptureRequest and CaptureResult apis are read only,
CaptureRequest.Builder has both getters and setters. It also
has an internal CameraMetadataNative object, on which  the getters and
setters can be called from different threads - even though we don't
officially say in the API spec that it is valid to do that.

A ReadWrite lock hasn't been used since in performance profiling there
doesn't seem to be a noticeable difference between using it and regular
synchronization with GCA. A ReadWrite lock would need explicit lock() and
unlock() calls, which may be more error prone.

Bug: 336882162
Flag: EXEMPT, 'synchronized' keyword being deleted from JNI calls can't
      be flagged

Test: GCA shows no global lock contention for metadata access
Test: GCA - basic validity
Test: Camera CTS

Change-Id: I22efb13c835388d4bb41a7a67e46b0215ef29e55
Signed-off-by: default avatarJayant Chowdhary <jchowdhary@google.com>
parent 3589bc5b

core/java/android/hardware/camera2/impl/CameraMetadataNative.java

+71 −44
Original line number Diff line number Diff line
@@ -437,7 +437,7 @@ public class CameraMetadataNative implements Parcelable { 
    }



    @Override

    public void writeToParcel(Parcel dest, int flags) {

    public synchronized void writeToParcel(Parcel dest, int flags) {

        nativeWriteToParcel(dest, mMetadataPtr);

    }
@@ -479,7 +479,7 @@ public class CameraMetadataNative implements Parcelable { 
        return getBase(key);

    }



    public void readFromParcel(Parcel in) {

    public synchronized void readFromParcel(Parcel in) {

        nativeReadFromParcel(in, mMetadataPtr);

        updateNativeAllocation();

    }
@@ -592,14 +592,16 @@ public class CameraMetadataNative implements Parcelable { 
    }



    private <T> T getBase(Key<T> key) {

        int tag;

        int tag, nativeType;

        byte[] values = null;

        synchronized (this) {

            if (key.hasTag()) {

                tag = key.getTag();

            } else {

                tag = nativeGetTagFromKeyLocal(mMetadataPtr, key.getName());

                key.cacheTag(tag);

            }

        byte[] values = readValues(tag);

            values = readValues(tag);

            if (values == null) {

                // If the key returns null, use the fallback key if exists.

                // This is to support old key names for the newly published keys.
@@ -613,7 +615,10 @@ public class CameraMetadataNative implements Parcelable { 
                }

            }



        int nativeType = nativeGetTypeFromTagLocal(mMetadataPtr, tag);

            nativeType = nativeGetTypeFromTagLocal(mMetadataPtr, tag);

        }

        // This block of code doesn't need to be synchronized since we aren't writing or reading

        // from the metadata buffer for this instance of CameraMetadataNative.

        Marshaler<T> marshaler = getMarshalerForKey(key, nativeType);

        ByteBuffer buffer = ByteBuffer.wrap(values).order(ByteOrder.nativeOrder());

        return marshaler.unmarshal(buffer);
@@ -1909,8 +1914,12 @@ public class CameraMetadataNative implements Parcelable { 
        setBase(key.getNativeKey(), value);

    }



    private <T> void setBase(Key<T> key, T value) {

        int tag;

    // The whole method needs to be synchronized since we're making

    // multiple calls to the native layer. From one call to the other (within setBase)

    // we expect the metadata's properties such as vendor id etc to

    // stay the same and as a result the whole method should be synchronized for safety.

    private synchronized <T> void setBase(Key<T> key, T value) {

        int tag, nativeType;

        if (key.hasTag()) {

            tag = key.getTag();

        } else {
@@ -1923,7 +1932,7 @@ public class CameraMetadataNative implements Parcelable { 
            return;

        } // else update the entry to a new value



        int nativeType = nativeGetTypeFromTagLocal(mMetadataPtr, tag);

        nativeType = nativeGetTypeFromTagLocal(mMetadataPtr, tag);

        Marshaler<T> marshaler = getMarshalerForKey(key, nativeType);

        int size = marshaler.calculateMarshalSize(value);
@@ -2126,7 +2135,7 @@ public class CameraMetadataNative implements Parcelable { 
        return true;

    }



    private void updateNativeAllocation() {

    private synchronized void updateNativeAllocation() {

        long currentBufferSize = nativeGetBufferSize(mMetadataPtr);



        if (currentBufferSize != mBufferSize) {
@@ -2209,6 +2218,11 @@ public class CameraMetadataNative implements Parcelable { 
    @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553)

    private long mMetadataPtr; // native std::shared_ptr<CameraMetadata>*



    // FastNative doesn't work with synchronized methods and we can do synchronization

    // wherever needed in the java layer (caller). At some places in java such as

    // setBase() / getBase(), we do need to synchronize the whole method, so leaving

    // synchronized out for these native methods.



    @FastNative

    private static native long nativeAllocate();

    @FastNative
@@ -2218,28 +2232,41 @@ public class CameraMetadataNative implements Parcelable { 


    @FastNative

    private static native void nativeUpdate(long dst, long src);

    private static synchronized native void nativeWriteToParcel(Parcel dest, long ptr);

    private static synchronized native void nativeReadFromParcel(Parcel source, long ptr);

    private static synchronized native void nativeSwap(long ptr, long otherPtr)

    @FastNative

    private static native void nativeWriteToParcel(Parcel dest, long ptr);

    @FastNative

    private static native void nativeReadFromParcel(Parcel source, long ptr);

    @FastNative

    private static native void nativeSwap(long ptr, long otherPtr)

            throws NullPointerException;

    @FastNative

    private static native void nativeSetVendorId(long ptr, long vendorId);

    private static synchronized native void nativeClose(long ptr);

    private static synchronized native boolean nativeIsEmpty(long ptr);

    private static synchronized native int nativeGetEntryCount(long ptr);

    private static synchronized native long nativeGetBufferSize(long ptr);

    @FastNative

    private static native void nativeClose(long ptr);

    @FastNative

    private static native boolean nativeIsEmpty(long ptr);

    @FastNative

    private static native int nativeGetEntryCount(long ptr);

    @FastNative

    private static native long nativeGetBufferSize(long ptr);



    @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553)

    private static synchronized native byte[] nativeReadValues(int tag, long ptr);

    private static synchronized native void nativeWriteValues(int tag, byte[] src, long ptr);

    private static synchronized native void nativeDump(long ptr) throws IOException; // dump to LOGD

    @FastNative

    private static native byte[] nativeReadValues(int tag, long ptr);

    @FastNative

    private static native void nativeWriteValues(int tag, byte[] src, long ptr);

    @FastNative

    private static native void nativeDump(long ptr) throws IOException; // dump to LOGD



    private static synchronized native ArrayList nativeGetAllVendorKeys(long ptr, Class keyClass);

    @FastNative

    private static native ArrayList nativeGetAllVendorKeys(long ptr, Class keyClass);

    @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553)

    private static synchronized native int nativeGetTagFromKeyLocal(long ptr, String keyName)

    @FastNative

    private static native int nativeGetTagFromKeyLocal(long ptr, String keyName)

            throws IllegalArgumentException;

    @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553)

    private static synchronized native int nativeGetTypeFromTagLocal(long ptr, int tag)

    @FastNative

    private static native int nativeGetTypeFromTagLocal(long ptr, int tag)

            throws IllegalArgumentException;

    @FastNative

    private static native int nativeGetTagFromKey(String keyName, long vendorId)
@@ -2257,7 +2284,7 @@ public class CameraMetadataNative implements Parcelable { 
     * @throws NullPointerException if other was null

     * @hide

     */

    public void swap(CameraMetadataNative other) {

    public synchronized void swap(CameraMetadataNative other) {

        nativeSwap(mMetadataPtr, other.mMetadataPtr);

        mCameraId = other.mCameraId;

        mHasMandatoryConcurrentStreams = other.mHasMandatoryConcurrentStreams;
@@ -2272,14 +2299,14 @@ public class CameraMetadataNative implements Parcelable { 
     *

     * @hide

     */

    public void setVendorId(long vendorId) {

    public synchronized void setVendorId(long vendorId) {

        nativeSetVendorId(mMetadataPtr, vendorId);

    }



    /**

     * @hide

     */

    public int getEntryCount() {

    public synchronized int getEntryCount() {

        return nativeGetEntryCount(mMetadataPtr);

    }
@@ -2288,7 +2315,7 @@ public class CameraMetadataNative implements Parcelable { 
     *

     * @hide

     */

    public boolean isEmpty() {

    public synchronized boolean isEmpty() {

        return nativeIsEmpty(mMetadataPtr);

    }
@@ -2307,7 +2334,7 @@ public class CameraMetadataNative implements Parcelable { 
     *

     * @hide

     */

    public <K>  ArrayList<K> getAllVendorKeys(Class<K> keyClass) {

    public synchronized <K> ArrayList<K> getAllVendorKeys(Class<K> keyClass) {

        if (keyClass == null) {

            throw new NullPointerException();

        }
@@ -2362,7 +2389,7 @@ public class CameraMetadataNative implements Parcelable { 
     *

     * @hide

     */

    public void writeValues(int tag, byte[] src) {

    public synchronized void writeValues(int tag, byte[] src) {

        nativeWriteValues(tag, src, mMetadataPtr);

    }
@@ -2377,7 +2404,7 @@ public class CameraMetadataNative implements Parcelable { 
     * @return {@code null} if there were 0 entries for this tag, a byte[] otherwise.

     * @hide

     */

    public byte[] readValues(int tag) {

    public synchronized byte[] readValues(int tag) {

        // TODO: Optimization. Native code returns a ByteBuffer instead.

        return nativeReadValues(tag, mMetadataPtr);

    }
@@ -2390,7 +2417,7 @@ public class CameraMetadataNative implements Parcelable { 
     *

     * @hide

     */

    public void dumpToLog() {

    public synchronized void dumpToLog() {

        try {

            nativeDump(mMetadataPtr);

        } catch (IOException e) {