duct-framework / duct

Server-side application framework for Clojure
MIT License
1.13k stars 51 forks source link

The Duct Repl start up time #62

Closed zerg000000 closed 7 years ago

zerg000000 commented 7 years ago

The following time is measured from my small project, it is quite painful when I need to restart my repl. I am wondering why the (require 'dev) would take over 13s to load and the initial go needs 10s to load....

nREPL server started on port 56605 on host 127.0.0.1 - nrepl://127.0.0.1:56605
(dev)
"Elapsed time: 13511.294624 msecs"
"Elapsed time: 0.005117 msecs"
=> :loaded
(time (go))
:duct.server.http.jetty/starting-server {:port 3000}
.....
"Elapsed time: 10055.75841 msecs"
(time (reset))
:reloading ()
"Elapsed time: 115.735037 msecs"
weavejester commented 7 years ago

Unfortunately this is a problem with Clojure that cannot be fixed by Duct. Loading a large number of Clojure files effectively means reading and compiling them, which equates to a significant initial overhead.

However, aside from adding new dependencies, you shouldn't ever need to restart the REPL.

zerg000000 commented 7 years ago

I would say it is not very good first impression when people try out frameworks. It is acceptable for long running repl. I would add the (go) to dev.clj, since the feeling of waiting 20s is better than waiting 10s -> (go) -> waiting 10s.

weavejester commented 7 years ago

There are times when I want (dev) but not (go), so I think I'll keep them separate. Unfortunately there's not a lot I can do about the time; but all Clojure frameworks will suffer from the same issue.

weavejester commented 7 years ago

Having said that, a (dev+go) shortcut might be useful...

zerg000000 commented 7 years ago

That's might be great! I experienced that, under a team environment, changes of sql files would always trigger a lot of restart across team members.

weavejester commented 7 years ago

Could you expand on that a little? Why would changing SQL files trigger a restart?

zerg000000 commented 7 years ago

It should be another issue. The start time issue could be closed.

Dev A: open repl, start development clj Dev B: open repl, start development cljs Dev A: modified the .up.sql, and shared to Dev B Dev B: pull without notice Dev B: (reset), the exception throws in ragtime

although Dev B only a cljs developer, he still suffer from the Dev A's sql changes

weavejester commented 7 years ago

Sorry, I'm not completely following. What exception is being thrown? Is the .down.sql file broken in some way? Doesn't fixing it and running (reset) again solve the problem?

zerg000000 commented 7 years ago

The .up.sql and .down.sql are modified (e.g. table is added), the hash is different from the hash stored ragtime_migrations. when (reset), the ragtime will roll back the previous schema and apply the new one, but the .down.sql could not be found since the hash is changed.

weavejester commented 7 years ago

Thanks for the clarification. So this occurs when someone pulls an updated migration before starting their REPL? What I don't understand is why restart the REPL?

zerg000000 commented 7 years ago

no one restarted their REPL, Dev B pull modified .up.sql + .down.sql and input (reset). Since the hash of .down.sql is changed, ragtime cannot find the .down.sql for rollback.

weavejester commented 7 years ago

Okay, I must have misunderstood your comment then:

I experienced that, under a team environment, changes of sql files would always trigger a lot of restart across team members.

What did you mean by "trigger a lot of restart across team members", if you didn't mean restarting the REPL?

zerg000000 commented 7 years ago

They opened the REPL and pulled the changes. Since the *.sql changed, the migrations hash are different and ragtime failed to find the rollback script. If reset failed to rollback/migrate the db, Dev A, Dev B need to clean up the db and ragtime's index in REPL. Most of the time, it means restart the REPL. They did not intended to restart the repl, but forced by exception.

(reset)
:reloading (re-frame.db re-frame.loggers re-frame.registrar re-frame.interceptor re-frame.cofx re-frame.utils re-frame.trace re-frame.subs re-frame.std-interceptors re-frame.events re-frame.router re-frame.fx re-frame.core buddy.auth.backend cljs.stacktrace dev bidi.bidi ajax.core xxx.handler.graphql xxx.middleware.auth xxx.main user)
:duct.migrator.ragtime/rolling-back :user.migration/create-user-table#d5b518cb
IllegalArgumentException No implementation of method: :run-down! of protocol: #'ragtime.protocols/Migration found for class: nil  clojure.core/-cache-protocol-fn (core_deftype.clj:583)
weavejester commented 7 years ago

Most of the time, it means restart the REPL. They did not intended to restart the repl, but forced by exception.

This is the part I don't understand. What's forcing them to restart the REPL?

I understand cleaning the database, but unless I'm forgetting something, I don't think there's any need to restart the REPL. The only thing that Ragtime adds to the REPL environment is the migration index, and because the identifiers include a hash, having some extra data in there shouldn't matter. Even if it did, a simple (clear) will remove it.

zerg000000 commented 7 years ago

I got what you mean, drop all tables + (clear). which (clear) is the missing piece of the puzzle. I don't know how to clean up the index. That's why restart REPL is needed.

weavejester commented 7 years ago

The Ragtime indexes have hashed IDs, so they're guaranteed to be unique. In theory this should mean clearing the system isn't necessary, but maybe there's a problem with that I haven't anticipated.

I've also just tried inputting (dev) (go) at the REPL, and to my surprise that works fine, so no need for a combined (dev+go).