Performance: short circuit form-decode-str about ring-codec HOT 7 CLOSED

In a separate PR, please.

from ring-codec.

If you can come up with an alternative implementation that's faster in most real time cases, and not significantly slower in edge cases, then sure. I'll need benchmarks, though!

from ring-codec.

Reading carefully the implementation of URLDecoder/decode looks like it only uses the encoding when it encounters hex characters, so the naive solution should be correct in all cases.
I'll upload detailed benchmarks tomorrow

from ring-codec.

Please find below the rationale, implementation and benchmark results

Rationale

0. Base implementation we compare against
1. Charset lookup incurs some overhead. We can cut it by def-ing the default charset and providing coercion
2. Decode only changes the string if it contains + or %. The theory is that checking they're contained in it is faster than calling into decode in the happy path, and the overhead is negligible in the sad path

Implementations:

1 - avoid charset lookup

(defn- to-charset
  ^Charset [s]
  (if (string? s)
    (Charset/forName s)
    s))

(def ^Charset utf-8 (to-charset "UTF-8"))

(defn form-decode-str'
  ([encoded]
   (form-decode-str' encoded utf-8))
  ([^String encoded encoding]
   (try
     (URLDecoder/decode encoded (to-charset encoding))
     (catch Exception _ nil))))

2 - avoid calling into decode unnecessarily

(defn form-decode-str''
  ([encoded]
   (form-decode-str'' encoded utf-8))
  ([^String encoded encoding]
   (try
     (if (or
          (.contains encoded "+")
          (.contains encoded "%"))
       (URLDecoder/decode encoded (to-charset encoding))
       encoded)
     (catch Exception _ nil))))

Methodology

• Use criterium.
• Input sizes with logarithmic size.
• Add the char which requires decoding at the end of the string, requiring a full scan

(def results
  (into
   []
   (for [n [1 2 4 8 16 32]
         :let [s (apply str (repeat n "a"))
               s' (str s "+")]]
     {:size n
      :no-decode0 (cc/quick-benchmark (form-decode-str s) nil)
      :no-decode1 (cc/quick-benchmark (form-decode-str' s) nil)
      :no-decode2 (cc/quick-benchmark (form-decode-str'' s) nil)
      :decode0 (cc/quick-benchmark (form-decode-str s') nil)
      :decode1 (cc/quick-benchmark (form-decode-str' s') nil)
      :decode2 (cc/quick-benchmark (form-decode-str'' s') nil)})))

Results

Timing is measured in ns:

Overhead between decode1 and decode2 should be only scanning the string before decoding it:

Based on these results, the full implementation (2) looks good to me. What do you think?

pngs rendered with plotly and clerk

from ring-codec.

Thank you for the detailed analysis. This looks like a good improvement to the existing implementation. Would you be able to open a PR, and link back to this issue?

from ring-codec.

Absolutely

While we're at it, would you like me to replace all instances where encoding is provided as String to Charset, or would you like that to be a different PR?

from ring-codec.

Since the form-decode-str PR could build on the charset PR, would you like the Charset PR first?
I can close #38 and split it

from ring-codec.