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

Commit 2f8bdb55 authored by Jeff Sharkey's avatar Jeff Sharkey
Browse files

Offer to write Strings through Parcels as UTF-8. 

Recently while investigating some Binder limits, I discovered that
we're still sending Strings across Binder as UTF-16, which is very
wasteful for two reasons:

1. The majority of data flowing through APIs like PackageManager is
already limited to US-ASCII, and by sending UTF-16 we're wasting
half of our transactions on null-byte overhead.

2. Internally ART is already "compressing" simple strings by storing
them as US-ASCII instead of UTF-16, meaning every time we want to
write a simple string to Binder, we're forced to first inflate it
to UTF-16.

This change first updates Parcel.cpp to accept char* UTF-8 strings,
similar to how it accepts char16_t* for UTF-16.  It then offers
both UTF-8 and UTF-16 variants to Parcel.java via JNI.  We also
update the String8 handling to behave identical to String16.

This change adds benchmarking to show that these new methods are
about 50% faster for US-ASCII strings, and about 68% faster for
complex strings that reference higher Unicode planes.  (So an
improvement in both cases!)

Bug: 154436100
Test: atest FrameworksCoreTests:ParcelTest
Test: make core-libart conscrypt okhttp bouncycastle vogar caliper && vogar --mode app_process --benchmark frameworks/base/core/tests/benchmarks/src/android/os/ParcelStringBenchmark.java
Change-Id: I3447a0ce28994421977ca50ba8902651f73372f4
parent c0867245

libs/binder/Parcel.cpp

+42 −30
Original line number Diff line number Diff line
@@ -987,12 +987,22 @@ status_t Parcel::writeCString(const char* str) 


status_t Parcel::writeString8(const String8& str)

{

    status_t err = writeInt32(str.bytes());

    // only write string if its length is more than zero characters,

    // as readString8 will only read if the length field is non-zero.

    // this is slightly different from how writeString16 works.

    if (str.bytes() > 0 && err == NO_ERROR) {

        err = write(str.string(), str.bytes()+1);

    return writeString8(str.string(), str.size());

}



status_t Parcel::writeString8(const char* str, size_t len)

{

    if (str == nullptr) return writeInt32(-1);



    status_t err = writeInt32(len);

    if (err == NO_ERROR) {

        uint8_t* data = (uint8_t*)writeInplace(len+sizeof(char));

        if (data) {

            memcpy(data, str, len);

            *reinterpret_cast<char*>(data+len) = 0;

            return NO_ERROR;

        }

        err = mError;

    }

    return err;

}
@@ -1832,37 +1842,39 @@ const char* Parcel::readCString() const 


String8 Parcel::readString8() const

{

    String8 retString;

    status_t status = readString8(&retString);

    if (status != OK) {

        // We don't care about errors here, so just return an empty string.

    size_t len;

    const char* str = readString8Inplace(&len);

    if (str) return String8(str, len);

    ALOGE("Reading a NULL string not supported here.");

    return String8();

}

    return retString;

}



status_t Parcel::readString8(String8* pArg) const

{

    int32_t size;

    status_t status = readInt32(&size);

    if (status != OK) {

        return status;

    }

    // watch for potential int overflow from size+1

    if (size < 0 || size >= INT32_MAX) {

        return BAD_VALUE;

    }

    // |writeString8| writes nothing for empty string.

    if (size == 0) {

    size_t len;

    const char* str = readString8Inplace(&len);

    if (str) {

        pArg->setTo(str, len);

        return 0;

    } else {

        *pArg = String8();

        return OK;

        return UNEXPECTED_NULL;

    }

}



const char* Parcel::readString8Inplace(size_t* outLen) const

{

    int32_t size = readInt32();

    // watch for potential int overflow from size+1

    if (size >= 0 && size < INT32_MAX) {

        *outLen = size;

        const char* str = (const char*)readInplace(size+1);

    if (str == nullptr) {

        return BAD_VALUE;

        if (str != nullptr) {

            return str;

        }

    pArg->setTo(str, size);

    return OK;

    }

    *outLen = 0;

    return nullptr;

}



String16 Parcel::readString16() const

libs/binder/include/binder/Parcel.h

+2 −0
Original line number Diff line number Diff line
@@ -119,6 +119,7 @@ public: 
    status_t            writeDouble(double val);

    status_t            writeCString(const char* str);

    status_t            writeString8(const String8& str);

    status_t            writeString8(const char* str, size_t len);

    status_t            writeString16(const String16& str);

    status_t            writeString16(const std::unique_ptr<String16>& str);

    status_t            writeString16(const char16_t* str, size_t len);
@@ -283,6 +284,7 @@ public: 
    const char*         readCString() const;

    String8             readString8() const;

    status_t            readString8(String8* pArg) const;

    const char*         readString8Inplace(size_t* outLen) const;

    String16            readString16() const;

    status_t            readString16(String16* pArg) const;

    status_t            readString16(std::unique_ptr<String16>* pArg) const;