Optimise HttpContentCodec#lookup

[guizmaii]

See also ghostdogpr/caliban#2366

[jdegoes]

Damn you're getting good. 😆

[guizmaii]

@jdegoes Thanks! I'm copying from some of the optimisations I see made by Kyri in ZIO! 🙂

We have a discussion with @987Nabil in Discord tho.
The default implementation of Map#getOrElse is instantiating an Option, which is annoying as it's in the hot path.
So I replaced the scala.Map by a scala.HashMap because its #getOrElse implementation doesn't instantiate any Option:

What's your opinion about that?

[987Nabil]

@jdegoes I was also wondering, if small HashMaps (4 or 5 elements max) are slower then the default map.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.27%. Comparing base (e9f1c02) to head (3ddc63d).
Report is 44 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3024      +/-   ##
==========================================
+ Coverage   64.78%   65.27%   +0.48%     
==========================================
  Files         157      155       -2     
  Lines        9395    10127     +732     
  Branches     1743     1886     +143     
==========================================
+ Hits         6087     6610     +523     
- Misses       3308     3517     +209     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kyri-petrou]

I might be missing something, but this looks like an ideal situation to use a mutable map instead of an immutable one with a var. What's the reasoning for using an immutable one?

[kyri-petrou]

I mentioned this on Discord but just putting this here as well for visibility; Most (if not all) immutable Map implementations override the getOrElse implementation on the base Map trait. E.g., Map1:

[987Nabil]

@kyri-petrou I don't think in this case it makes any significant difference between var or mutable. Because the writes are probably just a very few. This is read heavy

[kyri-petrou]

@987Nabil In that case I'd leave the implementation as is then (using Map instead of HashMap)

[kyri-petrou]

@guizmaii by the way I recently came across a small trick that might be good to apply here. To avoid the Function0 allocation in getOrElse, you can store the () => null in a val and do this:

val map: Map[String, String] = ???
private val orElseNull: () => String = () => null

map.getOrElse("foo", orElseNull())

This avoids the allocation because () => String and => String are the same thing.

Scala 3 only:
Store the val in the companion object and add the @static annotation to it

[kyri-petrou]

@guizmaii I just found out something interesting (or maybe a bug in dotty?). In Scala 2 this optimization works out of the box, but in Scala 3 it only works if you use the @static annotation 🤔

[guizmaii]

@kyri-petrou What is this for? Do I need it? 🤔

[guizmaii]

@987Nabil @kyri-petrou I made the changes you asked for :)

[987Nabil]

@guizmaii I think something of the optimizing is not working with scala js

[987Nabil]

@guizmaii I close PRs to have a better overview what is needed for 3.0
Feel free to reopen when it is building