[Mldonkey-users] upload priorities in 2.5.30.16

[Jason Baker]

I've noticed a couple bugs in the upload priority stuff in
daemon/common/commonUpload.ml:
   1. Priorities are used to decide whether to start downloading
      immediately, or go into the pending list.  But, priorities are
      not used to decide whether to start uploading to a pending
      client.
   2. Priorities are not adjusted by max_upload_slots, so this code
      will always push clients into the pending list.

I'm also less than satisfied with the way priorities are currently
defined.  Basically, "priorities" give each shared directory a number of
virtual upload slots, but do not express the notion that one directory
should have priority over another.

I would much rather see priorities defined s.t. for a priority p, at most 
max_upload_slots + p slots are used to upload from directories with
priority <= p.  The lowest priority directories will share the same
virtual upload slots, but higher priority directories will be able to
steal these slots.

I don't quite have this implemented for two reasons:
    1. the code I currently have does not take "stolen" slots into
       account. 
    2. some clients (listed as eDK) can call add_pending_slot with a
       source that has the default priority, then turn around and
       request a low priority source

It is easy to assign priorities in a way that some upload slots remain
available unless extremely rare files are requested.  When this
happens, I've noticed that I get an unusual number of upload requests,
and that clients stay in the pending list for an unusually long time.

Maybe priorities should be taken into account when reporting the
number of free slots to other clients.  Is that even possible?

For what it is worth, here is a patch to commonUploads.ml that
enforces priorities.

Jason

PS: Considering that uploaders and pending_slot_map tend to be very
small, wouldn't they be better implemented as simple lists than as
binary trees?

----------------------------------------------------------------------
--- commonUploads.ml~   Sat May 28 06:19:42 2005 
+++ commonUploads.ml    Mon Jul  4 13:46:38 2005 
@@ -877,7 +877,7 @@ 
 let ready_for_upload c = 
   Fifo.put upload_clients c 
      
-let add_pending_slot c = 
+let insert_pending_slot c = 
   if client_has_a_slot c then begin 
       if !verbose_upload then lprintf "Avoided inserting an uploader in pendin\
g slots!\n"; 
     end  
@@ -896,45 +896,59 @@ 
   if Intmap.mem (client_num c) !pending_slots_map then 
     pending_slots_map := Intmap.remove (client_num c) !pending_slots_map 
  
-let rec give_a_slot c = 
-  remove_pending_slot c; 
-  if not (client_is_connected c) then begin 
-      find_pending_slot () 
-    end 
-  else begin 
-      set_client_has_a_slot c true; 
-      client_enter_upload_queue c 
-    end 
-     
-and find_pending_slot () = 
-  try 
-    let rec iter () = 
-      let _, c = Intmap.top !pending_slots_map in 
-      give_a_slot c 
-    in 
-    iter () 
-  with _ -> () 
- 
+(** 
+ * Attempt to start uploading to client c.  If there are not enough 
+ * upload slots available at the shared file's priority, move c into 
+ * the pending list.  This function is only used by eDonkey code. 
+ * 
+ * Return true if the upload was started, and false if c remains 
+ * pending. 
+ * 
+ * This function assumes that share priorities have been normalized by 
+ * adding max_upload_slots.  These normalized priorities are 
+ * interpreted as the number of slots available to shared files of equal 
+ * or lesser priority.  Previous implementations of this function 
+ * treated the priority as the number of slots available to a 
+ * directory without defining any priority relationships between 
+ * directories. 
+ **) 
 let add_pending_slot c = 
-  let csh = client_upload c in 
-  let cdir = shared_dir csh in 
-  let cprio = ref (shared_prio csh) in 
-  (* if cdir <> "" then 
-    lprintf "Testing cdir %s\n" cdir; *) 
-  Intmap.iter (fun _ c ->  
-    let sh = client_upload c in 
-    if shared_dir sh = cdir then decr cprio 
-  ) !CommonClient.uploaders; 
-  (* if cdir <> "" then 
-    lprintf "Testing cprio %d\n" !cprio; *) 
-  if !cprio > 0 then begin 
-    remove_pending_slot c; 
-    if client_is_connected c then begin 
-      set_client_has_a_slot c true; 
-      client_enter_upload_queue c 
-    end 
-  end else 
-    add_pending_slot c 
+  ((*lprintf "add_pending_slot %d\n" (client_num c);*) 
+  if (client_is_connected c) then 
+      let csh = client_upload c in 
+         let cprio = shared_prio csh in 
+             let avail =  
+                 Intmap.fold (fun num c avail ->  
+                                  let sh = client_upload c in 
+                                      ((*lprintf "compare %d and %d\n" 
+                                       cprio (shared_prio sh);*) 
+                                       if (shared_prio sh) <= cprio then 
+                                           avail - 1 
+                                       else 
+                                           avail)) 
+                 !CommonClient.uploaders (cprio + !!max_upload_slots) 
+             in 
+                 (lprintf "client %d: prio = %d, avail = %d\n" 
+                             (client_num c) cprio avail; 
+                  if avail > 0 then  
+                      (remove_pending_slot c; 
+                       set_client_has_a_slot c true; 
+                       client_enter_upload_queue c; 
+                       true) 
+                  else 
+                      (insert_pending_slot c; 
+                       false)) 
+  else 
+      (remove_pending_slot c; false)) 
+ 
+let find_pending_slot () = 
+    (Intmap.fold (fun _ c looking -> 
+                    if looking then 
+                        not (add_pending_slot c) 
+                    else 
+                        looking) 
+                 !pending_slots_map true; 
+     ()) 
  
 let static_refill_upload_slots () = 
   let len = Intmap.length !CommonClient.uploaders in