# Optimization Recommendations

## 1. Async WebSocket Connection (Priority 1)

### Problem
Current implementation uses blocking connection wait:
```rust
while !connected_flag.load(Ordering::SeqCst) {
    std::thread::sleep(Duration::from_millis(100));
}
```

### Solution
Use async/await for non-blocking connection:

```rust
pub async fn connect_async(&mut self) -> Result<(), Error> {
    info!("Connecting websocket...");

    let connected_flag = Arc::new(AtomicBool::new(false));
    let flag_for_cb = Arc::clone(&connected_flag);

    let subscriptions = self.subscriptions.clone();

    let builder =
        self.client_builder
            .clone()
            .on("open", move |_payload: Payload, _socket: RawClient| {
                info!("Websocket connected");
                flag_for_cb.store(true, Ordering::SeqCst);
                // ... resubscription
            });

    let c = builder.connect()?;

    // Non-blocking wait
    tokio::time::sleep(Duration::from_millis(100)).await;
    while !connected_flag.load(Ordering::SeqCst) {
        tokio::time::sleep(Duration::from_millis(100)).await;
    }

    self.client = Some(c);
    Ok(())
}
```

**Expected Effect:**
- Does not block execution thread
- Better integration with async applications
- Ability to perform other operations during connection

---

## 2. Automatic Reconnection (Priority 1)

### Problem
No automatic reconnection on connection loss.

### Solution
Add connection supervisor with exponential backoff:

```rust
pub async fn run_with_reconnect(&mut self) {
    let mut reconnect_delay = Duration::from_secs(1);
    const MAX_DELAY: Duration = Duration::from_secs(60);
    
    loop {
        match self.connect_async().await {
            Ok(_) => {
                info!("Connected successfully");
                reconnect_delay = Duration::from_secs(1); // Reset delay
                
                // Wait for connection loss
                // Can add connection monitoring here
                tokio::time::sleep(Duration::from_secs(3600)).await;
            }
            Err(e) => {
                error!("Connection failed: {}, retrying in {:?}", e, reconnect_delay);
                tokio::time::sleep(reconnect_delay).await;
                
                // Exponential backoff with jitter
                reconnect_delay = std::cmp::min(
                    reconnect_delay * 2,
                    MAX_DELAY
                );
            }
        }
    }
}
```

**Expected Effect:**
- Automatic connection recovery
- Exponential backoff prevents server overload
- Improved fault tolerance

---

## 3. Caching Serialized Subscriptions (Priority 1 - Critical)

### Problem
On resubscription, `Value` to string conversion is performed for each channel:
```rust
Payload::from(sub.to_string())
```

### Solution
Store string representation of subscription instead of `Value`:

```rust
use std::sync::Arc;

#[derive(Clone)]
pub struct WsClient {
    client_builder: ClientBuilder,
    client: Option<Client>,
    subscriptions: Vec<Arc<str>>, // Change from Vec<Value> to Vec<Arc<str>> for maximum gain
    // Alternative: Vec<String> for simplicity (but smaller gain ~1.2-3x)
}

fn subscribe_with_product(&mut self, channel: &str, product_id: &str) {
    let message = ProductSubscriptionMessage {
        msg_type: channel.to_string(),
        product_id: product_id.to_string(),
    };

    let json_string = match serde_json::to_string(&message) {
        Ok(s) => Arc::from(s), // Use Arc<str> to avoid copying
        Err(e) => {
            error!("serialization_failed channel={channel} error={e}");
            return;
        }
    };
    self.subscriptions.push(json_string); // Without cloning content
}

// On resubscription:
for sub in subscriptions.iter() {
    _socket.emit("subscribe", Payload::from(sub.as_ref())); // Arc<str>::as_ref() cheaper than String::clone()
}
```

**Expected Effect:**
- Avoid repeated serialization on resubscription
- Fewer memory allocations
- Faster subscription recovery
- **Important:** Gain depends on implementation:
  - With `String::clone()`: ~1.2-3x faster (estimate, `String::clone()` still copies content)
  - With `Arc<str>` or `Bytes`: ~10-20x faster (estimate, avoids copying content)
- **Expected Gain (with `Arc<str>`): ~10-20x faster resubscription** (195.89 ns measured → ~10 ns estimate per channel; requires measurement after implementation)

---

## 4. Batching Resubscriptions (Priority 2 - High)

### Problem
On reconnection, separate message is sent for each channel.

### Solution
If API supports batching, send one command with all channels:

```rust
// In "open" callback:
if !subscriptions.is_empty() {
    // If API supports channel array
    let batch_message = serde_json::json!({
        "channels": subscriptions
    });
    _socket.emit("subscribe_batch", Payload::from(batch_message.to_string()));
} else {
    // Fallback to individual subscriptions
    for sub in subscriptions.iter() {
        _socket.emit("subscribe", Payload::from(sub.clone()));
    }
}
```

**Expected Effect:**
- Fewer network round-trips
- Faster subscription recovery
- Less server load

**Note:** Requires checking if API supports subscription batching.

---

## 5. Avoiding Cloning on Subscription Addition (Priority 3)

### Problem
On subscription addition, JSON value is cloned:
```rust
self.subscriptions.push(json_msg.clone());
```

### Solution
Use move instead of clone (already solved in recommendation 3, if storing strings).

If still using `Value`, can avoid cloning:
```rust
self.subscriptions.push(json_msg); // Move instead of clone
```

**Expected Effect:**
- Fewer memory allocations
- Better performance with large number of subscriptions

---

## 6. String Conversion Optimization (Priority 3)

### Problem
On subscription message creation, string conversion is performed:
```rust
msg_type: channel.to_string(),
product_id: product_id.to_string(),
```

### Solution
Use `Cow<str>` or accept `String` directly:

```rust
fn subscribe_with_product(&mut self, channel: String, product_id: String) {
    let message = ProductSubscriptionMessage {
        msg_type: channel, // Move instead of clone
        product_id: product_id,
    };
    // ...
}
```

Or use `Cow<str>` for flexibility:
```rust
use std::borrow::Cow;

fn subscribe_with_product(&mut self, channel: impl Into<Cow<'static, str>>, product_id: impl Into<Cow<'static, str>>) {
    let message = ProductSubscriptionMessage {
        msg_type: channel.into().into_owned(),
        product_id: product_id.into().into_owned(),
    };
    // ...
}
```

**Expected Effect:**
- Fewer allocations on subscription creation
- More flexible API

---

## 7. ClientBuilder Optimization (Priority 3)

### Problem
On callback registration, `ClientBuilder` is cloned:
```rust
let builder = self.client_builder.clone().on(channel, callback);
self.client_builder = builder;
```

### Solution
Use `Arc` for shared state or restructure architecture:

```rust
#[derive(Clone)]
pub struct WsClient {
    client_builder: Arc<ClientBuilder>, // Wrap in Arc
    client: Option<Client>,
    subscriptions: Vec<String>,
}

fn register_callback_internal<F>(&mut self, channel: &str, callback: F)
where
    F: Fn(Payload, RawClient) + Send + Sync + 'static,
{
    Arc::get_mut(&mut self.client_builder)
        .unwrap()
        .on(channel, callback);
    info!("Callback registered channel={channel}");
}
```

**Expected Effect:**
- Fewer allocations on callback registration
- More efficient memory usage

**Note:** Requires checking `ClientBuilder` API for support of mutable operations with `Arc`.

---

## 8. Async run_forever (Priority 3)

### Problem
`run_forever()` method uses blocking loop:
```rust
pub fn run_forever(&self) {
    loop {
        std::thread::sleep(Duration::from_secs(60));
    }
}
```

### Solution
Use async version:

```rust
pub async fn run_forever_async(&self) {
    loop {
        tokio::time::sleep(Duration::from_secs(60)).await;
        // Can add connection check here
    }
}
```

**Expected Effect:**
- Does not block execution thread
- Better integration with async applications
- Ability to perform other operations

---

## Implementation Prioritization

### Phase 1 (Critical - Immediately)
1. ✅ Async WebSocket Connection
2. ✅ Automatic Reconnection

### Phase 2 (High - Soon)
3. ✅ Caching Serialized Subscriptions
4. ✅ Batching Resubscriptions (if API supports)

### Phase 3 (Medium - When Possible)
5. ✅ Avoiding Cloning on Subscription Addition
6. ✅ String Conversion Optimization
7. ✅ ClientBuilder Optimization
8. ✅ Async run_forever

## Testing Recommendations

After implementing each optimization, it is recommended:

1. **Run benchmarks** to measure improvements:
   ```bash
   cargo bench
   ```

2. **Test Functionality:**
   - Ensure all subscriptions work correctly
   - Check reconnection on connection loss
   - Check error handling

3. **Load Testing:**
   - Testing with large number of subscriptions (10, 50, 100+)
   - Testing frequent reconnections
   - Testing high message frequency

4. **Performance Monitoring:**
   - Measure connection time
   - Measure reconnection time
   - Measure memory usage

## Expected Improvements

After implementing all optimizations, the following improvements are expected (all values are estimates and require measurement after implementation):

- **Resubscription Time:** reduction by ~10-20x through caching with `Arc<str>` (195.89 ns → ~10 ns per channel, measured → estimate)
  - With `String::clone()`: reduction by ~1.2-3x (estimate)
- **Subscription Cloning Time:** reduction by ~10-20x through using `Vec<Arc<str>>` instead of `Vec<Value>` (10.284 µs for 50 subscriptions → ~0.5 µs, measured → estimate)
  - With `Vec<String>`: reduction by ~1.2-3x (estimate)
- **Network Round-trips on Resubscription:** reduction by 10-100x through batching (if API supports)
- **Connection Latency:** reduction by ~50ms on average through eliminating busy-wait polling (analytical estimate: average overshoot = interval/2 = 50ms)
- **Memory Usage:** reduction through avoiding `Value` cloning (estimate, requires measurement)
