refactor(adb): pre-encode packet ID as number in sync command

libraries/adb/src/commands/sync/list.ts

@@ -17,8 +17,7 @@ export interface AdbSyncEntry extends AdbSyncStat {
 export const AdbSyncEntryResponse = new Struct({ littleEndian: true })
     .concat(AdbSyncLstatResponse)
     .uint32("nameLength")
-    .string("name", { lengthField: "nameLength" })
-    .extra({ id: AdbSyncResponseId.Entry as const });
+    .string("name", { lengthField: "nameLength" });
 
 export type AdbSyncEntryResponse =
     (typeof AdbSyncEntryResponse)["TDeserializeResult"];
@@ -26,8 +25,7 @@ export type AdbSyncEntryResponse =
 export const AdbSyncEntry2Response = new Struct({ littleEndian: true })
     .concat(AdbSyncStatResponse)
     .uint32("nameLength")
-    .string("name", { lengthField: "nameLength" })
-    .extra({ id: AdbSyncResponseId.Entry2 as const });
+    .string("name", { lengthField: "nameLength" });
 
 export type AdbSyncEntry2Response =
     (typeof AdbSyncEntry2Response)["TDeserializeResult"];

libraries/adb/src/commands/sync/pull.ts

@@ -8,8 +8,7 @@ import type { AdbSyncSocket } from "./socket.js";
 
 export const AdbSyncDataResponse = new Struct({ littleEndian: true })
     .uint32("dataLength")
-    .uint8Array("data", { lengthField: "dataLength" })
-    .extra({ id: AdbSyncResponseId.Data as const });
+    .uint8Array("data", { lengthField: "dataLength" });
 
 export type AdbSyncDataResponse =
     (typeof AdbSyncDataResponse)["TDeserializeResult"];
@@ -52,6 +51,7 @@ export function adbSyncPull(
     socket: AdbSyncSocket,
     path: string,
 ): ReadableStream<Uint8Array> {
+    // TODO: use `ReadableStream.from` when it's supported
     return new PushReadableStream(async (controller) => {
         for await (const data of adbSyncPullGenerator(socket, path)) {
             await controller.enqueue(data);

libraries/adb/src/commands/sync/push.ts

@@ -111,7 +111,7 @@ export interface AdbSyncPushV2Options extends AdbSyncPushV1Options {
 }
 
 export const AdbSyncSendV2Request = new Struct({ littleEndian: true })
-    .uint32("id", placeholder<AdbSyncRequestId>())
+    .uint32("id")
     .uint32("mode")
     .uint32("flags", placeholder<AdbSyncSendV2Flags>());
 

libraries/adb/src/commands/sync/request.ts

@@ -2,21 +2,23 @@ import Struct from "@yume-chan/struct";
 
 import { encodeUtf8 } from "../../utils/index.js";
 
-export enum AdbSyncRequestId {
-    List = "LIST",
-    ListV2 = "LIS2",
-    Send = "SEND",
-    SendV2 = "SND2",
-    Lstat = "STAT",
-    Stat = "STA2",
-    LstatV2 = "LST2",
-    Data = "DATA",
-    Done = "DONE",
-    Receive = "RECV",
+import { adbSyncEncodeId } from "./response.js";
+
+export namespace AdbSyncRequestId {
+    export const List = adbSyncEncodeId("LIST");
+    export const ListV2 = adbSyncEncodeId("LIS2");
+    export const Send = adbSyncEncodeId("SEND");
+    export const SendV2 = adbSyncEncodeId("SND2");
+    export const Lstat = adbSyncEncodeId("STAT");
+    export const Stat = adbSyncEncodeId("STA2");
+    export const LstatV2 = adbSyncEncodeId("LST2");
+    export const Data = adbSyncEncodeId("DATA");
+    export const Done = adbSyncEncodeId("DONE");
+    export const Receive = adbSyncEncodeId("RECV");
 }
 
 export const AdbSyncNumberRequest = new Struct({ littleEndian: true })
-    .string("id", { length: 4 })
+    .uint32("id")
     .uint32("arg");
 
 export interface AdbSyncWritable {
@@ -25,9 +27,13 @@ export interface AdbSyncWritable {
 
 export async function adbSyncWriteRequest(
     writable: AdbSyncWritable,
-    id: AdbSyncRequestId | string,
+    id: number | string,
     value: number | string | Uint8Array,
 ): Promise<void> {
+    if (typeof id === "string") {
+        id = adbSyncEncodeId(id);
+    }
+
     if (typeof value === "number") {
         await writable.write(
             AdbSyncNumberRequest.serialize({ id, arg: value }),
@@ -39,9 +45,8 @@ export async function adbSyncWriteRequest(
         value = encodeUtf8(value);
     }
 
-    // `writable` will copy inputs to an internal buffer,
-    // so we write header and `buffer` separately,
-    // to avoid an extra copy.
+    // `writable` is buffered, it copies inputs to an internal buffer,
+    // so don't concatenate headers and data here, that will be an unnecessary copy.
     await writable.write(
         AdbSyncNumberRequest.serialize({ id, arg: value.byteLength }),
     );

libraries/adb/src/commands/sync/response.ts

@@ -1,3 +1,4 @@
+import { getUint32LittleEndian } from "@yume-chan/no-data-view";
 import type {
     AsyncExactReadable,
     StructLike,
@@ -7,16 +8,34 @@ import Struct from "@yume-chan/struct";
 
 import { decodeUtf8 } from "../../utils/index.js";
 
-export enum AdbSyncResponseId {
-    Entry = "DENT",
-    Entry2 = "DNT2",
-    Lstat = "STAT",
-    Stat = "STA2",
-    Lstat2 = "LST2",
-    Done = "DONE",
-    Data = "DATA",
-    Ok = "OKAY",
-    Fail = "FAIL",
+function encodeAsciiUnchecked(value: string): Uint8Array {
+    const result = new Uint8Array(value.length);
+    for (let i = 0; i < value.length; i += 1) {
+        result[i] = value.charCodeAt(i);
+    }
+    return result;
+}
+
+/**
+ * Encode ID to numbers for faster comparison
+ * @param value A 4-character string
+ * @returns A 32-bit integer by encoding the string as little-endian
+ */
+export function adbSyncEncodeId(value: string): number {
+    const buffer = encodeAsciiUnchecked(value);
+    return getUint32LittleEndian(buffer, 0);
+}
+
+export namespace AdbSyncResponseId {
+    export const Entry = adbSyncEncodeId("DENT");
+    export const Entry2 = adbSyncEncodeId("DNT2");
+    export const Lstat = adbSyncEncodeId("STAT");
+    export const Stat = adbSyncEncodeId("STA2");
+    export const Lstat2 = adbSyncEncodeId("LST2");
+    export const Done = adbSyncEncodeId("DONE");
+    export const Data = adbSyncEncodeId("DATA");
+    export const Ok = adbSyncEncodeId("OKAY");
+    export const Fail = adbSyncEncodeId("FAIL");
 }
 
 export class AdbSyncError extends Error {}
@@ -30,18 +49,24 @@ export const AdbSyncFailResponse = new Struct({ littleEndian: true })
 
 export async function adbSyncReadResponse<T>(
     stream: AsyncExactReadable,
-    id: AdbSyncResponseId,
+    id: number | string,
     type: StructLike<T>,
 ): Promise<T> {
-    const actualId = decodeUtf8(await stream.readExactly(4));
-    switch (actualId) {
+    if (typeof id === "string") {
+        id = adbSyncEncodeId(id);
+    }
+
+    const buffer = await stream.readExactly(4);
+    switch (getUint32LittleEndian(buffer, 0)) {
         case AdbSyncResponseId.Fail:
             await AdbSyncFailResponse.deserialize(stream);
             throw new Error("Unreachable");
         case id:
             return await type.deserialize(stream);
         default:
-            throw new Error(`Expected '${id}', but got '${actualId}'`);
+            throw new Error(
+                `Expected '${id}', but got '${decodeUtf8(buffer)}'`,
+            );
     }
 }
 
@@ -49,12 +74,16 @@ export async function* adbSyncReadResponses<
     T extends Struct<object, PropertyKey, object, unknown>,
 >(
     stream: AsyncExactReadable,
-    id: AdbSyncResponseId,
+    id: number | string,
     type: T,
 ): AsyncGenerator<StructValueType<T>, void, void> {
+    if (typeof id === "string") {
+        id = adbSyncEncodeId(id);
+    }
+
     while (true) {
-        const actualId = decodeUtf8(await stream.readExactly(4));
-        switch (actualId) {
+        const buffer = await stream.readExactly(4);
+        switch (getUint32LittleEndian(buffer, 0)) {
             case AdbSyncResponseId.Fail:
                 await AdbSyncFailResponse.deserialize(stream);
                 throw new Error("Unreachable");
@@ -70,7 +99,7 @@ export async function* adbSyncReadResponses<
                 break;
             default:
                 throw new Error(
-                    `Expected '${id}' or '${AdbSyncResponseId.Done}', but got '${actualId}'`,
+                    `Expected '${id}' or '${AdbSyncResponseId.Done}', but got '${decodeUtf8(buffer)}'`,
                 );
         }
     }

libraries/adb/src/commands/sync/stat.ts

@@ -29,7 +29,6 @@ export const AdbSyncLstatResponse = new Struct({ littleEndian: true })
     .int32("size")
     .int32("mtime")
     .extra({
-        id: AdbSyncResponseId.Lstat as const,
         get type() {
             return (this.mode >> 12) as LinuxFileType;
         },
@@ -83,7 +82,6 @@ export const AdbSyncStatResponse = new Struct({ littleEndian: true })
     .uint64("mtime")
     .uint64("ctime")
     .extra({
-        id: AdbSyncResponseId.Stat as const,
         get type() {
             return (this.mode >> 12) as LinuxFileType;
         },

libraries/adb/src/commands/sync/sync.ts

@@ -9,6 +9,7 @@ import type { AdbSyncEntry } from "./list.js";
 import { adbSyncOpenDir } from "./list.js";
 import { adbSyncPull } from "./pull.js";
 import { adbSyncPush } from "./push.js";
+import type { AdbSyncSocketLocked } from "./socket.js";
 import { AdbSyncSocket } from "./socket.js";
 import type { AdbSyncStat, LinuxFileType } from "./stat.js";
 import { adbSyncLstat, adbSyncStat } from "./stat.js";
@@ -150,7 +151,7 @@ export class AdbSync extends AutoDisposable {
      */
     async write(options: AdbSyncWriteOptions): Promise<void> {
         if (this.needPushMkdirWorkaround) {
-            // It may fail if the path is already existed.
+            // It may fail if `filename` already exists.
             // Ignore the result.
             // TODO: sync: test push mkdir workaround (need an Android 8 device)
             await this._adb.subprocess.spawnAndWait([
@@ -167,6 +168,10 @@ export class AdbSync extends AutoDisposable {
         });
     }
 
+    lockSocket(): Promise<AdbSyncSocketLocked> {
+        return this._socket.lock();
+    }
+
     override async dispose() {
         super.dispose();
         await this._socket.close();

libraries/android-bin/src/pm.ts

@@ -273,10 +273,17 @@ export class PackageManager extends AdbCommandBase {
             PACKAGE_MANAGER_INSTALL_OPTIONS_MAP,
         );
         if (!options?.skipExisting) {
-            // Today `pm` defaults to replace existing application (`-r` will be ignored),
-            // but old versions defaults to skip existing application (like `-R`, but obviously
-            // they didn't have this switch and ignores it if present).
-            // If `skipExisting` is not set, add `-r` to ensure compatibility with old versions.
+            /*
+             * | behavior             | previous version     | modern version       |
+             * | -------------------- | -------------------- | -------------------- |
+             * | replace existing app | requires `-r`        | default behavior [1] |
+             * | skip existing app    | default behavior [2] | requires `-R`        |
+             *
+             * [1]: `-r` recognized but ignored)
+             * [2]: `-R` not recognized but ignored
+             *
+             * So add `-r` when `skipExisting` is `false` for compatibility.
+             */
             args.push("-r");
         }
         return args;