Created attachment 644256 [details] [diff] [review]
There's a pretty big SS date-format-tofte regression due to not adding scripts to the eval cache (regression from d6531ef05a6f).
Need to land this on aurora too.
Comment on attachment 644256 [details] [diff] [review]
Just remembered Brian is gone for the weekend so changing reviewer.
I see why d6531ef05a6f needed to not persist the EvalCacheLookup. However, I don't see why the AddPtr p_ had to go. Given that relookupOrAdd (using the AddPtr) avoids hashing the whole script again, it seems beneficial. (On that subject Jan, does your attached patch recover all the slowdown?)
Unfortunately, there isn't a bug to refer to, Bill do you remember the hazard?
(In reply to Luke Wagner [:luke] from comment #3)
> (On that subject Jan, does your attached patch recover all the slowdown?)
I got 15 ms on SS date-format-tofte before revision d6531ef05a6f and at tip with the patch. 15 ms is not much so maybe a micro-benchmark will show a difference. Okay if I land the attached patch to fix the regression and we file a follow-up bug to avoid hashing the script twice?
(In reply to Jan de Mooij (:jandem) from comment #4)
Of course, land away. I was more just asking to understand if there was a real reason.
Oh, one request: can you turn the lookupForAdd+add into put?
Fixed by backout of changeset d6531ef05a6f.
(In reply to David Mandelin [:dmandelin] from comment #7)
> Fixed by backout of changeset d6531ef05a6f.
What about Aurora 16?
Also backed out of 16.