Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Global vs Local Variables #39

Closed
latenitefilms opened this issue Dec 5, 2016 · 17 comments
Closed

Global vs Local Variables #39

latenitefilms opened this issue Dec 5, 2016 · 17 comments
Assignees
Labels
Milestone

Comments

@latenitefilms
Copy link
Contributor

@randomeizer - As discussed on Skype, here's that discussion I was talking about in regards to global vs local variables.

@randomeizer
Copy link
Contributor

Hmm, interesting.

I think a better way to manage it would be to have our 'private' variables being assigned as fields of the module it is relevant to. For example, with the 'clipboard.lua' file now, instead of this:

local log = hs.logger.new("clipboard")

I would try something like this:

local mod = {}

mod.log = hs.logger.new("clipboard")

-- other code goes here

return mod

The main downside is that you then have to add mod. in front of your variable names. So, from this:

log.d("A debug message!")

to this:

mod.log.d("A debug message!")

Not the end of the world, and I believe it would resolve the GC issue without making everything a global, which potentially would introduce clashes between different files.

@latenitefilms
Copy link
Contributor Author

I'm not sure that solves the issue though...

As a general recommendation, I would try to avoid having anything in your init.lua be a local - the init.lua's scope ends at when it's been executed, so anything marked local is a viable candidate for Garbage Collection.

Please correct me if I'm wrong, but as init.lua finishes execution as soon as loadScript() is done, basically ALL the local variables such as all the watchers, are now up for Garbage Collection. Based on what I've read, the only way to solve this is to make sure that all the watchers, timers, etc. are Global Variables - so they avoid Garbage Collection.

Does that make sense?

@randomeizer
Copy link
Contributor

We may have to make the 'fcpxHacks' module global. But if all the other variables are on that module, they shouldn't get GC'd because they are still linked to the fcpxHacks module.

It will require some testing to see if it actually works. I'll have a look into it.

@randomeizer randomeizer self-assigned this Dec 5, 2016
randomeizer added a commit that referenced this issue Dec 5, 2016
@latenitefilms
Copy link
Contributor Author

Really awesome work @randomeizer !

Not sure if you've seen my fumbling around yet, but eventually I'd like to move all the "Final Cut Pro" commands into the one module - so that if anyone else wants to build something similar to FCPX Hacks, they've got an easy starting point. I've started doing this really roughly for #45.

@randomeizer
Copy link
Contributor

Yeah, I'd like to split up the different functional parts into separate files, similar to 'clipboard.lua', for example. It will help with keeping the code readable and manageable.

@randomeizer
Copy link
Contributor

I'm thinking that we should maybe do the modularisation through individual issue tags though, and maybe wait until I've done the initial work here in #39, or we may end up stepping on each other's toes. My goal in this one is to make sure everything is safely in a global, or linked as a sub-property in fcpxHacks, which should have the same effect. Then we can start reorganising stuff.

randomeizer added a commit that referenced this issue Dec 6, 2016
@randomeizer
Copy link
Contributor

FYI, I decided to switch to using 'hs.xxx' to reference built-in Hammerspoon libraries. It's kind of redundant creating a tonne of globals for those and/or assigning them as 'mod.xxx', when it's just three extra characters to reference the built-in globals directly.

This won't work with the non-standard extensions (touchbar, etc), but will be fine for most things we use.

@latenitefilms
Copy link
Contributor Author

Yeah, I'd like to split up the different functional parts into separate files, similar to 'clipboard.lua', for example. It will help with keeping the code readable and manageable.

Agreed! I was originally against it, as I thought it would slow me down, or slow the code down, but in retrospect, I think it's a really great idea. Thank you!

I'm thinking that we should maybe do the modularisation through individual issue tags though, and maybe wait until I've done the initial work here in #39, or we may end up stepping on each other's toes.

Yep - agreed. The only stuff I've added in #45 thus far is now plist functions, so it shouldn't affect anything.

FYI, I decided to switch to using 'hs.xxx' to reference built-in Hammerspoon libraries. It's kind of redundant creating a tonne of globals for those and/or assigning them as 'mod.xxx', when it's just three extra characters to reference the built-in globals directly.

Ummm... I'm not sure about that one. I believe there's a very slight speed increase by calling it locally. This speed increase was actually noticeable, when I was originally trying to get the GUI Scripting stuff to work.

In regards to Global vs Local, have a read of this and let me know what you think!

randomeizer added a commit that referenced this issue Dec 6, 2016
…he global function defined in the fcpxhacks/init.lua
@randomeizer
Copy link
Contributor

We would be making the globals either way. The only difference is whether you make an additional table property lookup by adding the hs. first. For one-off calls, I'd be very surprised if it made a difference. If it was multiple calls inside a loop, maybe.

If there are specific tasks that are measurably slower, we can optimise those. But in general, I think this is a better way.

Can you recall the specific activities that were slower?

@latenitefilms
Copy link
Contributor Author

Sorry to be a pain, but I'd much prefer it if the code didn't have "hs." everywhere. I feel like it just makes things messy for no real benefit. I liked it the way it was, because it allowed to code things really quickly as easily. Sorry - I know it's only three characters of extra typing, but I'm lazy, and I like things to look as simple as possible.

In terms of speed, to dig up an older conversation...

There is a school of thought that says that writing your code so that any module you need is stored in a local variable is faster... technically this is true because local variables are searched before global variables are, and looking for hs.hotkey.bind for example means that the search occurs as follows: local variables are searched for hs. It's not found, so the global name space is searched for hs. It's found, then hotkey is searched for in the hs table. Then bind is searched for in the hotkey table of the hs table. If you had typed local hotkey = require("hs.hotkey") and then tried to use hotkey.bind, then the local variables are searched for hotkey. When found the hotkey table is searched for bind -- 2 searches rather than 4.

I say "technically" in the previous paragraph because on a modern computer, the speed difference is measured in micro or even nano seconds. Unless you have something that is iterating millions of times, this won't be what's causing your script to be slow. But, every little bit does add up, so... YMMV.

I personally like to require everything in my scripts and put them into local variables because I think it's cleaner. I see an autoload statement as an example of me forgetting or mistyping something in my script; but that's me. Others like the convenience of having one consistent name for something that never changes in any of their scripts... when you store a module's reference in a local variable, you pick the variable name, so if I'm inconsistent in my naming, inspect in one script might refer to a completely different module or function in another.

So in terms of speed, it probably doesn't make any substantial difference, but I guess technically it's faster (in micro/nano seconds!) - so I reckon we just do it.

Sorry!

@randomeizer
Copy link
Contributor

The thing is, when you do this:

alert										= require("hs.alert")

you are making them globals, not locals. There are two issues with this:

  1. the minor speed difference you are quoting (and believe me, it is incredibly minor) is not achieved anyway, because it's a global.
  2. You get potential clashes between different scripts loading a 'require' value into the same global.

For example, in our project, we do the above in /hs/fcpxhacks/init.lua and also in /hs/fcpxhacks/module/fcpx10-3.lua. Both of them are applying the require to the exact same alert global value - one overwrites the other.

This is typically not such a big issue for our 'requires', because they would have a fairly standard name, and it's loading the same library. But it is potentially a much bigger one for variables being used inside a module, since it's not completely obvious what globals a given module is defining.

To me, having 'hs.' in front of standard Hammerspoon class calls is actually making the code clearer. It clearly says, this is a standard library, and doesn't have any dependency on global variables which may be defined in other modules.

@latenitefilms
Copy link
Contributor Author

No worries! Well, rather than making them global, would you have any objections if we made them local to each LUA script? For example:

local alert = require("hs.alert")

In retrospect, I'm not even sure why I made all these "extensions" global in the first place! I think it was because I couldn't get this to work:

local window.filter = require("hs.window.filter")

...so I just said, "screw it", and made them all global.

@randomeizer
Copy link
Contributor

Making them local would help. The fix for the problem you mentioned, assuming you are also requiring 'hs.window', would be this:

local window = require("hs.window")
window.filter = require("hs.window.filter")

However, you may find that window.filter will just work anyway - not sure though. Haven't tested that.

@latenitefilms
Copy link
Contributor Author

Let's do that then... let's just make them local, so I can be happy not typing "hs." all the time.

Thanks for all your help @randomeizer !

Once you're done with issue #39, I'll update fcpxhacks/init.lua and fcpx10-3.lua to start using the hs.finalcutpro module for all the Final Cut Pro specific stuff (which I've already started building in #45).

Hopefully once all this "plumbing" stuff is done, I can get back to adding new features again!

@randomeizer
Copy link
Contributor

So, I did some benchmarking. Ran a simple operation (hs.hash.MDF("Foobar")) 1 million times using each access method (hs.hash, globalHash = require..., and local localHash = require...), then ran the whole batch of tests 5 times to see if there was any major variations. Here's the result:

Benchmarking hs/global/local access speeds

Round 1:
hs.hash.MD5('Foobar') x 1000000: 7925.97ms total/0.00792597ms avg
globalHash.MD5('Foobar') x 1000000: 7870.862ms total/0.007870862ms avg
localHash.MD5('Foobar') x 1000000: 8093.153ms total/0.008093153ms avg
Round 2:
hs.hash.MD5('Foobar') x 1000000: 7882.957ms total/0.007882957ms avg
globalHash.MD5('Foobar') x 1000000: 7905.412ms total/0.007905412ms avg
localHash.MD5('Foobar') x 1000000: 7802.362ms total/0.007802362ms avg
Round 3:
hs.hash.MD5('Foobar') x 1000000: 7887.1ms total/0.0078871ms avg
globalHash.MD5('Foobar') x 1000000: 7935.96ms total/0.00793596ms avg
localHash.MD5('Foobar') x 1000000: 7884.049ms total/0.007884049ms avg
Round 4:
hs.hash.MD5('Foobar') x 1000000: 7976.648ms total/0.007976648ms avg
globalHash.MD5('Foobar') x 1000000: 8035.403ms total/0.008035403ms avg
localHash.MD5('Foobar') x 1000000: 7869.933ms total/0.007869933ms avg
Round 5:
hs.hash.MD5('Foobar') x 1000000: 7975.87ms total/0.00797587ms avg
globalHash.MD5('Foobar') x 1000000: 8005.033ms total/0.008005033ms avg
localHash.MD5('Foobar') x 1000000: 7973.864ms total/0.007973864ms avg

So, running each operation 1 million times took about 7.8 to 8.1 seconds, depending. Each operation was taking around of 0.008 milliseconds (aka 8 nanoseconds) to run. Which method was fastest varied on each run, although the globalHash was most consistently the slowest (except for the first one, go figure). But by slowest, we are talking about a variation of 0.2 nanoseconds, which honestly, is probably a symptom of what other activities my computer happened to be doing at that moment in time.

The upshot is, performance-wise, it doesn't matter whether we use hs.xxx, a global, or a local. It's purely about making a choice that makes the code safest and easiest to work with.

Here's the benchmark code, for your reference:

-- local/global/hs speed tests

function benchmarkAccess(count)
	
	print("Round "..count..":")

	local clock = os.clock
	local max = 1000000
	local start, result

	-- Use global hs. namespace
	start = clock()
	for i=1,max do
		hs.hash.MD5("Foobar")
	end
	result = clock()-start
	print("hs.hash.MD5('Foobar') x "..max..": "..(result*1000).."ms total/"..(result/max*1000).."ms avg")

	-- Use a global from require
	globalHash = require("hs.hash")
	start = clock()
	for i=1,max do
		globalHash.MD5("Foobar")
	end
	result = clock()-start
	print("globalHash.MD5('Foobar') x "..max..": "..(result*1000).."ms total/"..(result/max*1000).."ms avg")

	-- Use a global from require
	local localHash = require("hs.hash")
	start = clock()
	for i=1,max do
		localHash.MD5("Foobar")
	end
	result = clock()-start
	print("localHash.MD5('Foobar') x "..max..": "..(result*1000).."ms total/"..(result/max*1000).."ms avg")
end

print("Benchmarking hs/global/local access speeds\n")

for j=1,5 do
	benchmarkAccess(j)
end

I just pasted it into the end of the ~/.hammerspoon/init.lua to run it.

@randomeizer
Copy link
Contributor

Ok, locals they are then.

@latenitefilms
Copy link
Contributor Author

Legend, thanks mate!

randomeizer added a commit that referenced this issue Dec 7, 2016
…d to a module, as appropriate.

* Made ‘require’ imports locals.
* Renamed the clipboard local values to ‘clipboard.xxx’
randomeizer added a commit that referenced this issue Dec 7, 2016
…priate function if the did not to be module-wide variables.
@randomeizer randomeizer added this to the 0.70 milestone Dec 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants