Fork me on GitHub
#clojure-dev
<
2020-03-03
>
ikitommi08:03:17

As ring is heading towards using namespaced keys in request & response maps (https://github.com/ring-clojure/ring/issues/393), is there a plan to support namespaces keys with records?

ikitommi08:03:39

I guess the lookups for unqualified & qualified keys from maps should have about the same performance?

ikitommi08:03:01

a quick test for a field lookup for a ring-request map (14 keys) using: 1. qualfied key 24ns 2. unqualified key 20ns 3. record with unqualified key 4ns

dominicm22:03:45

Ran this test with a structmap for my own comparison. Structmap with key: (:foo/bar x) 24ns Structmap with accessor: (bar x) 11.65ns Record: (:bar x) 15.83ns

dominicm22:03:00

Ran 999999 times in a dotimes block to compare.

ikitommi08:03:37

if not record, maybe some new map-like type which would enable fast lookups with known qualified keys?

Alex Miller (Clojure team)13:03:19

given how much things get added to request maps, I don't think records are necessarily a good choice anyways - changing a record means instantiating a new record

Alex Miller (Clojure team)13:03:56

there are many ways request maps are used - focusing only on key lookup performance is missing other parts of the picture

Alex Miller (Clojure team)13:03:26

it seems weird, but you may actually want to look at struct-maps

Alex Miller (Clojure team)13:03:04

although I'm not sure struct-maps support ns'ed keys well either, I don't know if I've ever used them

seancorfield16:03:05

They do not. clojure.java.jdbc used to use them years ago and switched to regular hash maps. Someone did some perf tests on next.jdbc early on and suggested I switch from regular hash maps to struct maps for improved performance but the lack of qualified keys was a blocker for that. Using a record for a result set row is a faster option (but also doesn't support qualified keys).

dominicm22:03:01

They seem to work for me, although they print wrong:

user=> (defstruct bar :bar/example)
#'user/bar
user=> (def example (accessor bar :bar/example))
#'user/example
user=> (struct bar "foobar")
#:bar{:bar/example nil, :example "foobar"}
user=> (keys (struct bar "foobar"))
(:bar/example)
user=> (example (struct bar "foobar"))
"foobar"
Is it worth me opening a ask/jira about the print bug?

dominicm22:03:47

The bug starts in lift-ns. It creates an empty of the struct, which still includes the key as nil, ofc. And also includes the ns-removed version of the key. Simple answers are to convert structs to hashmap before printing, or to have lift-ns not use empty but instead an empty hashmap.

seancorfield23:03:17

Interesting. I don't remember what tests I ran but I know I didn't get anything like the results I wanted so I came away thinking you couldn't use qualified keys at all in structs... Clearly, I didn't test extensively 🙂

Alex Miller (Clojure team)23:03:13

That is actually already logged as it affects sorted maps

Alex Miller (Clojure team)23:03:22

I’ve spent some time looking at it and it’s annoying

Alex Miller (Clojure team)13:03:12

a customized map-like data structure optimized for known usage patterns may make sense here. I don't know that records are the right answer. getting a clearer picture for the operations and their frequency is the first step though

ikitommi10:03:04

Started to investigate this. Never used struct-maps, but good time to understand how they work (even if not suitable for this). My interest is in optimizing the read part and now think the best way to do it is a mix of protocols, lazy evaluation and a map-like custom type to make it ring-compatible. Thanks for the pointers.

ikitommi10:03:29

the current full-copy request with ring/takes 2.4µs on my MacBook. Goal is get into <200ns for apps that only need anything but the non-normalized info (like headers) and a <50ns for the benchmark case of just uri & request-method.

ikitommi10:03:09

writing the whole request into log will be as slow as full-copy, can't help with that.

ikitommi10:03:36

memory allocation is also really important, need to test that too.

bronsa14:03:49

changing a record means instantiating a new record how's that different from changing a map?

bronsa14:03:02

perf differences of copying an array vs accessing all of a record's fields?

Alex Miller (Clojure team)14:03:43

yeah, it's at least something that should be considered

bronsa14:03:56

makes sense

bronsa16:03:52

@alexmiller dunno if you get notified about jira comments (I don't anymore), but resolve introduces a bug

bronsa16:03:20

if we don't want to open up the compiler, we could do it in a similar way as we do it for defn

bronsa16:03:24

(if (clojure.lang.Util/equals nil (clojure.lang.Compiler$HostExpr/maybeSpecialTag tag))
                                (let [c (clojure.lang.Compiler$HostExpr/maybeClass tag false)]

Alex Miller (Clojure team)16:03:31

you should get notified, but some of the "created by"/"updated by" metadata was broken in the migration - by default you should get notified if you created it, are watching, or voted for it

bronsa16:03:07

I may not receive the emails I care about anymore, but Atlassian makes sure I get those sweet sweet spam ones :)

😛 4
bronsa16:03:14

is it possible to go back to receiving emails about issues created on my projects? I looked everywhere and couldn't find how to

Alex Miller (Clojure team)16:03:48

I can probably fix that, will look at it later

Alex Miller (Clojure team)21:03:54

I've updated the notification scheme both for your projects and many others to have project lead notification like we had on the old jira

Alex Miller (Clojure team)21:03:05

I looked at your settings and some of the project settings. in general, you should receive updates if a) you reported the issue, b) you are the assignee, or c) you are "watching" the issue. Now, in the projects where you are a lead, you should also get updates on issues in your projects.

Alex Miller (Clojure team)21:03:56

if you find a specific issue and a specific event that you don't get an email for, let me know and they have a tool that can use all that info and tell me what notifications you should get and why

bronsa09:03:46

thanks a lot!

bronsa16:03:50

user=> (defprotocol p (^longs f [_]))
p
user=> (set! *warn-on-reflection* true)
true
user=> (defn foo [x] (aget (f x) 0))
Syntax error (IllegalArgumentException) compiling . at (REPL:1:15).
Unable to resolve classname: #'clojure.core/longs

Alex Miller (Clojure team)16:03:53

sorry, you prob put it on the ticket

Alex Miller (Clojure team)16:03:18

seems like there should be tests to expose this

seancorfield16:03:05

They do not. clojure.java.jdbc used to use them years ago and switched to regular hash maps. Someone did some perf tests on next.jdbc early on and suggested I switch from regular hash maps to struct maps for improved performance but the lack of qualified keys was a blocker for that. Using a record for a result set row is a faster option (but also doesn't support qualified keys).

dominicm22:03:01

They seem to work for me, although they print wrong:

user=> (defstruct bar :bar/example)
#'user/bar
user=> (def example (accessor bar :bar/example))
#'user/example
user=> (struct bar "foobar")
#:bar{:bar/example nil, :example "foobar"}
user=> (keys (struct bar "foobar"))
(:bar/example)
user=> (example (struct bar "foobar"))
"foobar"
Is it worth me opening a ask/jira about the print bug?